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

Deprecate requirements harder #3659

Merged
merged 1 commit into from Jan 18, 2018

Conversation

Projects
None yet
6 participants
@MikeMcQuaid
Member

MikeMcQuaid commented Jan 10, 2018

Remove more Requirement logic to enable future removal of default formula logic. Also, output deprecations, convert symbol requirement usage to deps and simplify the compatibility code for the direct Requirement usage.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:deprecated-requirements-harder branch from 592d533 to 87d5d4e Jan 10, 2018

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:deprecated-requirements-harder branch 2 times, most recently from 1ac0481 to 7de12b2 Jan 10, 2018

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jan 11, 2018

Will merge this shortly before a 1.5.0 tag.

@ilovezfs

This comment has been minimized.

Contributor

ilovezfs commented Jan 15, 2018

This causes

Error: Invalid formula: /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core/Formula/pipenv.rb
uninitialized constant Language::Python

on all the virtualenv formulae.

It's odd that it doesn't cause any test failures here.

@ilovezfs

This comment has been minimized.

Contributor

ilovezfs commented Jan 15, 2018

Also some of the deprecation notices aren't correct. For example,


Calling 'depends_on :mpi' is deprecated!
Use 'depends_on "open-mpi" => [:cc, :cxx, :recommended]' instead.

should say


Calling 'depends_on :mpi' is deprecated!
Use 'depends_on "open-mpi" => :recommended' instead.

and

Calling 'depends_on :ruby' is deprecated!
Use 'depends_on "ruby" => [:1.9, :build]' instead.

should say

Calling 'depends_on :ruby' is deprecated!
Use 'depends_on "ruby" => :build' instead.
@GauthamGoli

This comment has been minimized.

Member

GauthamGoli commented Jan 15, 2018

These deprecations are also detected by Custom Cops.
Any reason why we have them at two different places ?

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jan 15, 2018

@GauthamGoli That's runtime vs. audit/style time checks. We could arguable kill some of the audit/style checks after at least the odisabled code kicks in.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:deprecated-requirements-harder branch from 7de12b2 to 0721563 Jan 15, 2018

@MikeMcQuaid

This comment has been minimized.

Member

MikeMcQuaid commented Jan 15, 2018

@ilovezfs Issues should be addressed, thanks for the testing.

javian added a commit to Homebrew/homebrew-php that referenced this pull request Jan 16, 2018

abstract-php: remove requirement usage.
This will be deprecated in Homebrew/brew#3659.

Closes #4709.

Signed-off-by: Jan Viljanen <527069+javian@users.noreply.github.com>

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:deprecated-requirements-harder branch 2 times, most recently from 76a8c08 to 2393619 Jan 16, 2018

Deprecate requirements harder
Remove more Requirement logic to enable future removal of default
formula logic. Also, output deprecations, convert symbol requirement
usage to deps and simplify the compatibility code for the direct
Requirement usage.

@MikeMcQuaid MikeMcQuaid force-pushed the MikeMcQuaid:deprecated-requirements-harder branch from 2393619 to e5c82dd Jan 18, 2018

@MikeMcQuaid MikeMcQuaid merged commit e67b745 into Homebrew:master Jan 18, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@MikeMcQuaid MikeMcQuaid deleted the MikeMcQuaid:deprecated-requirements-harder branch Jan 18, 2018

@sjackman

This comment has been minimized.

Contributor

sjackman commented on Library/Homebrew/compat/requirements.rb in e5c82dd Jan 19, 2018

Is "'depends_on \"cvs\"'" correct? The warning message output by brew tap is nonetheless correct.

Calling 'depends_on :python' is deprecated!
Use 'depends_on "python"' instead.

Note that it doesn't agree in every case in this file except CVSRequirement.

This comment has been minimized.

Contributor

alyssais replied Jan 20, 2018

I don’t think this is correct. Wanna PR?

This comment has been minimized.

Member

MikeMcQuaid replied Jan 20, 2018

Yeh, not correct, fixed in #3706

@maelvalais

This comment has been minimized.

tex depends on cvs? I guess it is a copy-paste typo :)

Side note: what is the 'new' procedure for requiring latex (e.g. through an option --with-latex)? Now that it is deprecated, do we have to do the requirement logic inside the formula.rb?

Thanks!

This comment has been minimized.

Member

MikeMcQuaid replied Jan 20, 2018

Side note: what is the 'new' procedure for requiring latex (e.g. through an option --with-latex)?

https://docs.brew.sh/Building-Against-Non-Homebrew-Dependencies.html

This comment has been minimized.

Contributor

ilovezfs replied Jan 20, 2018

The copy-paste thing is fixed in #3706

Side note: what is the 'new' procedure for requiring latex (e.g. through an option --with-latex)? Now that it is deprecated, do we have to do the requirement logic inside the formula.rb?

If it's a 3rd party tap, you can use env :std in the formula.

This comment has been minimized.

maelvalais replied Jan 20, 2018

Thanks for the link and env :std tip! I am indeed maintaining a tap and I was (secretly) wishing to merge the formula into homebrew-core. I just removed the depends_on :tex (using pre-built in order to skip the latex part) but the formula is 769 lines long because of ~50 Perl vendored dependencies 😞

This comment has been minimized.

Contributor

ilovezfs replied Jan 20, 2018

You'll probably want to create a mini CPAN instead as znapzend did oetiker/znapzend@73af9cd

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.