Skip to content
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.

Sign up for GitHub

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

Jump to bottom

missing_formula: add message for Asymptote #5816

Merged
merged 5 commits into from
Mar 13, 2019
Merged

missing_formula: add message for Asymptote #5816

merged 5 commits into from
Mar 13, 2019

Conversation

jmarshall
Copy link
Contributor

Hoping to install Asymptote produces

$ brew install asymptote
Error: No available formula with the name "asymptote" 
==> Searching for a previously deleted formula (in the last month)...
Error: No previously deleted formula found.
==> Searching for similarly named formulae...
Error: No similarly named formulae found.
==> Searching taps...
==> Searching taps on GitHub...
Error: No formulae found in taps.

After further searching I came across Homebrew/legacy-homebrew#23029, which removed the previous asymptote formula on the basis that it was better to install it via MacTeX. This patch helps future users to side-step that searching for information stage:

$ brew install asymptote
Updating Homebrew...
Error: No available formula with the name "asymptote" 
Asymptote is bundled with MacTeX. Install it via TeX Live Utility or:
  tlmgr install asymptote
  • 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.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew tests with your changes locally?

@jmarshall
missing_formula: add message for Asymptote
0c7c45a 
Per Homebrew/legacy-homebrew#23029, there is no formula for Asymptote
because you may well already have it installed via MacTeX anyway.
@jmarshall
Clarify that TeX Live Utility (tlmgr) comes with the TeX installation
a702273 
MacTeX is the macOS package of TeX Live; also mention the latter for
the sake of non-Mac Brew.
MikeMcQuaid
MikeMcQuaid reviewed Mar 11, 2019
View reviewed changes
Library/Homebrew/missing_formula.rb
@@ -26,6 +26,10 @@ def blacklisted_reason(name)
Minimal installation:
brew cask install basictex
EOS
when "asymptote" then <<~EOS
Asymptote is part of MacTeX:
brew cask install mactex
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmarshall Is this an accurate statement? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not especially. s/MacTeX/MacTeX distribution/ or /MacTeX collection/ would help. But mostly it's incomplete as to the casks available (as previously noted in #5816 (comment)) and to choices made during pkg installation as to how much of the collection actually gets installed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jmarshall As I think I've explained above: if a new entry is added for Asymptote I want the user to be provided with a brew cask invocation that will install it. If brew cask install mactex will not do so: what will?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I've explained above, the existing missing_formula.rb entry for tex/latex/etc already contains an accurate list of the TeX-related casks available. The single cask install you've added is incomplete information; something like what I've pushed would be more accurate (you might prefer brew search).

As a Brew user looking for Asymptote, what I actually needed was (1) a pointer to the MacTeX that I already have installed, (2) a hint that the command is called asy, and (3, if I were new to MacTeX and/or had installed a cut-down MacTeX subset) a pointer to TeX Live Utility. What I didn't need was being reminded how to type brew [cask] install mactex.

Hence the PR as at c508bda. However (2) and (3) might be considered out of Brew's scope, so if something like the current bfee3be772fa1bd224fea2da35e67d81a3b82bb8 makes the Homebrew maintainers happy that is fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to be explicit about what is needed:

  • I need this PR to point to a single brew cask install which will install Asymptote for it to be merged.
  • I need confirmation on whether brew cask install mactex will install Asymptote?

If you're unwilling or unable to do either of these: please close this PR, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would have been helpful to be explicit that that was what your concern was about.

Asymptote is installed by the mactex and mactex-no-gui casks, but not by basictex. (It is also of course installed by basictex+tlmgr install asymptote or by installing the upstream MacTeX package, which are options that had been covered by previous versions of this PR.)

So if the Homebrew maintainers want to adjust this to some variant of brew cask install mactex{,-no-gui} on merging, please do so.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're unwilling or unable to do either of these: please close this PR, thanks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Closing this PR as a result. This has been a frustrating experience; please address maintainers feedback without arguing in hopeful future PRs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're unwilling or unable to do either of these: please close this PR, thanks.

I spent several hours doing those things yesterday, pal.

  • You completely rewrote the text of this PR once, so I anticipated you would want to do so again with your preferred cask text.

  • I spent several hours breaking my TeX installation to confirm which casks contained Asymptote.

Thank you for your time. If I should happen to raise future PRs, I hope they will be less frustrating experiences for all concerned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent several hours doing those things yesterday, pal.
I need this PR to point to a single brew cask install which will install Asymptote for it to be merged.

You did not do that.

  • You completely rewrote the text of this PR once, so I anticipated you would want to do so again with your preferred cask text.

I've restored it to that text as it is accurate (based on what you said above) and meets the requirements (a single brew cask install invocation).

Thank you for your time.

And for yours.

If I should happen to raise future PRs, I hope they will be less frustrating experiences for all concerned.

The documentation helps cover some of this:

Do not argue with Homebrew maintainers. You may disagree but unless they change their mind, please implement what they request. Ultimately they control what is included in Homebrew, as they have to support any changes that are made.
https://github.com/Homebrew/brew/blob/master/docs/How-To-Open-a-Homebrew-Pull-Request.md

@MikeMcQuaid MikeMcQuaid reopened this Mar 13, 2019
@jmarshall
Copy link
Contributor Author

jmarshall commented Mar 13, 2019 •
edited
Loading

I have had further thoughts about what a useful message for this tiny thing is, and will be filing a new PR.

@jmarshall jmarshall closed this Mar 13, 2019
@MikeMcQuaid MikeMcQuaid reopened this Mar 13, 2019
@MikeMcQuaid MikeMcQuaid merged commit 056da81 into Homebrew:master Mar 13, 2019
@MikeMcQuaid
Copy link
Member

Thanks for your work on this PR @jmarshall!

@lock lock bot added the outdated PR was locked due to age label Apr 12, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Apr 12, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MikeMcQuaid MikeMcQuaid MikeMcQuaid left review comments

Assignees
No one assigned
Labels
outdated PR was locked due to age
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jmarshall @MikeMcQuaid