Hint at new location of migrated formulae #1732

zmwangx · 2016-12-26T00:01:52Z

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 tests with your changes locally?

This is a partial implementation of https://github.com/Homebrew/brew-evolution/pull/15. Please read the proposal, which was basically agreed upon. (Note that #1371 was also a step towards the goal.)

With this change:

$ brew info aeskeyfind
Error: No available formula with the name "aeskeyfind"
==> Searching for migrated formulae...
A deprecated formula named "aeskeyfind" has been migrated from homebrew/core to homebrew/boneyard.
A formula named "aeskeyfind" has been migrated from zmwangx/old to zmwangx/new.

(zmwangx/old has a placeholder tap_migrations.json with {"aeskeyfind": "zmwangx/new"}.)

$ brew install aeskeyfind
Error: No available formula with the name "aeskeyfind"
==> Searching for migrated formulae...
A deprecated formula named "aeskeyfind" has been migrated from homebrew/core to homebrew/boneyard.
A formula named "aeskeyfind" has been migrated from zmwangx/old to zmwangx/new.

$ brew info r
Error: No available formula with the name "r"
==> Searching for migrated formulae...
A formula named "r" has been migrated from caskroom/cask to homebrew/science.

$ brew install r
Error: No available formula with the name "r"
==> Searching for migrated formulae...
A formula named "r" has been migrated from caskroom/cask to homebrew/science.

$ brew info aeskeyfin
Error: No available formula with the name "aeskeyfin"
==> Searching for migrated formulae...

$ brew install aeskeyfin
Error: No available formula with the name "aeskeyfin"
==> Searching for migrated formulae...
==> Searching for similarly named formulae...
Error: No similarly named formulae found.
==> Searching taps...
Error: No formulae found in taps.

MikeMcQuaid

I really like this approach. A few comments but I suspect we'll get this merged soon.

MikeMcQuaid · 2016-12-29T12:20:35Z

Library/Homebrew/cmd/info.rb

How long does search_for_migrated_formula take to run? I wonder if we can skip this message.

It doesn't take long, could be left out. I put it in just so that we print a message for each kind of search.

MikeMcQuaid · 2016-12-29T12:20:43Z

Library/Homebrew/cmd/install.rb

How long does search_for_migrated_formula take to run? I wonder if we can skip this message.

MikeMcQuaid · 2016-12-29T12:21:44Z

Library/Homebrew/migrated.rb

Probably worth putting this in Library/Homebrew/migrator.rb

Also, make print_messages part of an options hash.

MikeMcQuaid · 2016-12-29T12:22:42Z

Library/Homebrew/migrated.rb

Probably want to use EOS.undent and a few lines here. Test on a 80 character wide terminal and see how it looks.

MikeMcQuaid · 2016-12-29T12:22:50Z

Library/Homebrew/migrated.rb

next unless print_messages

MikeMcQuaid · 2017-01-25T09:34:36Z

@zmwangx Ping on these comments. If you don't have the desire/time to address them that's fine and I'll do them inline at some point.

zmwangx · 2017-01-25T16:29:28Z

@MikeMcQuaid Sorry, I forgot about this. Just addressed most comments. Two outstanding comments:

Probably worth putting this in Library/Homebrew/migrator.rb

migrator.rb is for renames though, which is pretty much unrelated.

Probably want to use EOS.undent and a few lines here. Test on a 80 character wide terminal and see how it looks.

The difficulty here is that messages could vary in length. I could, sort of, break them like

A deprecated formula named "aeskeyfind" has been migrated
from homebrew/core to homebrew/boneyard.
A formula named "aeskeyfind" has been migrated
from zmwangx/old to zmwangx/new.

but then it looks weird, especially on a reasonably wide terminal (including a 80-column one). I'd say soft wrap is better than hard wrap here.

MikeMcQuaid · 2017-01-25T16:30:48Z

Related to this in case it piques your interest is gracefully handling the case where there hasn't been a migration based just on the Git history. Also worth noting #1812 if you haven't seen it already.

zmwangx · 2017-01-25T17:00:33Z

gracefully handling the case where there hasn't been a migration based just on the Git history.

Um, if the formula can't be found and is also not in git history, how do we print anything useful? On the other hand, if something appears in tap_migrations.json (case handled here) it must have been migrated, regardless of whether git history is deep enough, no? Could you please give an example (made-up is fine) of the case you're talking about?

Also worth noting #1812 if you haven't seen it already.

Thanks for the pointer. Yeah, it would be nice to issue a warning in #1812's case, whether the formula is migrated or not. Not sure what's the right way to issue a warning there though. As long as there's a warning, search_for_migrated_formula can piggyback onto that.

zmwangx · 2017-01-25T17:06:36Z

By the way,

brew tests --no-compat

is failing because I wrote

module Homebrew
   def search_for_migrated_formula
   end
end

instead of

module Homebrew
   def self.search_for_migrated_formula
   end
end

and the former requires method_missing from compat/global.rb. I think I've seen the former pattern in the code base though; I guess they're not tested. What should I do in this case?

MikeMcQuaid · 2017-01-26T13:43:18Z

other hand, if something appears in tap_migrations.json (case handled here) it must have been migrated, regardless of whether git history is deep enough, no? Could you please give an example (made-up is fine) of the case you're talking about?

Sure. We're planning on removing the boneyard and generally want to handle cases when formulae are deleted rather than migrated better. If I remove aeskeyfind from tap_migrations.rb it would be cool to do the equivalent of git log --max-count=1 -- Formula/aeskeyfind.rb (which can be a little slow) to look in the local Git history to see if it exists there. If this is a shallow repository it'll be quicker. In that case we can say that the formula was removed from Homebrew but they can view it for adding to e.g. their own tap with git show ${REMOVE_COMMIT}^:Formula/aeskeyfind.rb

Yeah, it would be nice to issue a warning in #1812's case, whether the formula is migrated or not.

What would you suggest?

What should I do in this case?

Use self. or module_function?

MikeMcQuaid · 2017-02-12T17:13:45Z

Still definitely interested in this PR when you can get it 💚, @zmwangx.

Sure. We're planning on removing the boneyard and generally want to handle cases when formulae are deleted rather than migrated better. If I remove aeskeyfind from tap_migrations.rb it would be cool to do the equivalent of git log --max-count=1 -- Formula/aeskeyfind.rb (which can be a little slow) to look in the local Git history to see if it exists there. If this is a shallow repository it'll be quicker. In that case we can say that the formula was removed from Homebrew but they can view it for adding to e.g. their own tap with git show ${REMOVE_COMMIT}^:Formula/aeskeyfind.rb

There's logic for this now in #1996 which can either be duped in here or integrated after it's merged.

zmwangx · 2017-02-13T03:08:55Z

@MikeMcQuaid Sorry, I forgot about this PR again.

I just added support for searching for a deleted formula in git history, based on logic very similar to that of #1996. I have renamed migrated.rb to historic.rb, because it contains functions to search for historic formulae. #1996 can actually be rewritten to take advantage of search_for_deleted_formula in historic.rb.

Here are some samples (I removed aeskeyfind from tap_migrations.json):

$ brew info aeskeyfind
Error: No available formula with the name "aeskeyfind"
==> Searching among deleted formulae...
aeskeyfind was deleted from homebrew/core in commit effe183400.
Run `brew boneyard aeskeyfind` to show the formula's content prior to its removal.

$ brew info homebrew/binary/adobe-air-sdk-flex
Error: No available formula with the name "homebrew/binary/adobe-air-sdk-flex"
==> Searching among deleted formulae...
homebrew/binary/adobe-air-sdk-flex was deleted from homebrew/binary in commit 88edc0f.
Run `brew boneyard homebrew/binary/adobe-air-sdk-flex` to show the formula's content prior to its removal.

Unfortunately, brew tests is showing this error from inside parallel_tests:

/usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:53:in `pwd': No such file or directory - getcwd (Errno::ENOENT)
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:53:in `message'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:45:in `log'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:15:in `log_test_run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:80:in `run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/minitest-5.10.1/lib/minitest.rb:158:in `block in __run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/minitest-5.10.1/lib/minitest.rb:158:in `map'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/minitest-5.10.1/lib/minitest.rb:158:in `__run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/minitest-5.10.1/lib/minitest.rb:135:in `run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/parallel_tests-2.13.0/lib/parallel_tests/test/runtime_logger.rb:90:in `run'
	from /usr/local/Homebrew/Library/Homebrew/test/vendor/bundle/ruby/2.0.0/gems/minitest-5.10.1/lib/minitest.rb:62:in `block in autorun'

Smells like a bug in parallel_tests, somehow triggered by (possibly) a bug in my code, unless I'm doing something obviously wrong. Rather annoying because the error messages don't tell me anything.

zmwangx · 2017-02-13T03:14:07Z

To answer an earlier question:

Yeah, it would be nice to issue a warning in #1812's case, whether the formula is migrated or not.

What would you suggest?

Something like "Warning: formula foo not found in any tap / tap blah, loaded from keg instead"?

By the way, I'm happy to keep working on this PR, but please feel free to take over at any time if you feel like it.

MikeMcQuaid · 2017-02-13T08:16:31Z

Something like "Warning: formula foo not found in any tap / tap blah, loaded from keg instead"?

Yeh, something like this would be good.

By the way, I'm happy to keep working on this PR, but please feel free to take over at any time if you feel like it.

Thanks 👍

MikeMcQuaid · 2017-03-18T15:40:15Z

I've made a bunch of changes here and think it's good to merge. Interested in thoughts from @reitermarkus, @zmwangx and @ilovezfs.

Once this is merged I think we can also delete the homebrew/boneyard as the messaging is sufficiently good now.

zmwangx · 2017-03-18T15:51:51Z

LGTM.

reitermarkus · 2017-03-18T21:31:29Z

Library/Homebrew/missing_formula.rb

This is a good place for module_function.

I can't get module_function to work with the extend and alias here. I think we tried before on Emoji and gave up.

Which extend?

Sorry, not extend, the require and using macOS specific stuff at the bottom of the file.

reitermarkus · 2017-03-18T21:33:03Z

Library/Homebrew/missing_formula.rb

Maybe call this blacklist_reason?

reitermarkus · 2017-03-18T21:33:44Z

Library/Homebrew/test/missing_formula_spec.rb

:: instead of .

reitermarkus · 2017-03-18T21:40:40Z

Library/Homebrew/extend/os/mac/missing_formula.rb

Use Formatter.url here, and a period at the end.

reitermarkus · 2017-03-18T21:41:35Z

Library/Homebrew/missing_formula.rb

Maybe call this reason?

reitermarkus · 2017-03-19T22:46:30Z

Library/Homebrew/test/missing_formula_spec.rb

:: instead of .

Partial implementation of https://github.com/Homebrew/brew-evolution/pull/15, along with the ability to search for deleted formulae in git history (inspired by Homebrew#1996) which is not described in the proposal. See also: Homebrew#1371.

This makes it easier to turn an arbitrary path into a tap path.

This wasn’t adapted to the new, multiple repository world.

This will allow extending this class so it can be used by more than just blacklisting.

These methods belong together so combine them in a single class to provide a simpler API.

MikeMcQuaid · 2017-03-21T17:31:38Z

Thanks for getting this started @zmwangx!

zmwangx force-pushed the hint-migrations branch from 97649cc to 07eda2e Compare December 26, 2016 07:52

MikeMcQuaid requested changes Dec 29, 2016

View reviewed changes

zmwangx force-pushed the hint-migrations branch from 07eda2e to f0a2872 Compare January 25, 2017 16:24

zmwangx force-pushed the hint-migrations branch 3 times, most recently from 00c4b89 to 444d92c Compare February 13, 2017 02:55

zmwangx mentioned this pull request Feb 13, 2017

brew boneyard: show contents of deleted formula. #1996

Closed

MikeMcQuaid force-pushed the hint-migrations branch from 444d92c to 4d224b6 Compare March 18, 2017 15:39

MikeMcQuaid approved these changes Mar 18, 2017

View reviewed changes

ilovezfs approved these changes Mar 18, 2017

View reviewed changes

MikeMcQuaid force-pushed the hint-migrations branch 2 times, most recently from d7883db to 640b16c Compare March 18, 2017 15:43

reitermarkus reviewed Mar 18, 2017

View reviewed changes

MikeMcQuaid force-pushed the hint-migrations branch 2 times, most recently from bea79eb to 349bb6c Compare March 19, 2017 16:23

MikeMcQuaid mentioned this pull request Mar 19, 2017

brew outdated can trigger auto-tapping of the boneyard #1607

Closed

reitermarkus reviewed Mar 19, 2017

View reviewed changes

Library/Homebrew/test/missing_formula_spec.rb Outdated

Copy link

Member

reitermarkus Mar 19, 2017

Choose a reason for hiding this comment

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

:: instead of .

MikeMcQuaid force-pushed the hint-migrations branch from 349bb6c to 0ccb8a3 Compare March 20, 2017 07:54

zmwangx and others added 6 commits March 20, 2017 18:20

Tap: add from_path helper method.

ac10b2a

This makes it easier to turn an arbitrary path into a tap path.

search: tweak specific formula match formatting.

8cedd62

cmd/log: improve output messaging.

623c95b

This wasn’t adapted to the new, multiple repository world.

blacklist: move to missing_formula class instead.

80e95b6

This will allow extending this class so it can be used by more than just blacklisting.

missing_formula: subsume historic logic.

f59eb35

These methods belong together so combine them in a single class to provide a simpler API.

MikeMcQuaid force-pushed the hint-migrations branch from ecf26e8 to f59eb35 Compare March 20, 2017 19:37

MikeMcQuaid merged commit 4117d19 into Homebrew:master Mar 21, 2017

zmwangx deleted the hint-migrations branch March 21, 2017 17:32

zmwangx mentioned this pull request Mar 22, 2017

missing_formula: warn when git-log takes very long #2377

Merged

5 tasks

This was referenced Mar 22, 2017

Import homebrew/dupes into core. Homebrew/homebrew-core#11282

Closed

Bulk import from homebrew/versions (7/7) Homebrew/homebrew-core#10863

Closed

Bulk import from homebrew/versions (4/7) Homebrew/homebrew-core#10860

Closed

ilovezfs mentioned this pull request May 24, 2017

imageworsener 1.3.2 Homebrew/homebrew-core#13896

Closed

4 tasks

Homebrew locked and limited conversation to collaborators May 3, 2018

Uh oh!

Hint at new location of migrated formulae #1732

Hint at new location of migrated formulae #1732

Uh oh!

Conversation

zmwangx commented Dec 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid commented Jan 25, 2017

Uh oh!

zmwangx commented Jan 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Jan 25, 2017

Uh oh!

zmwangx commented Jan 25, 2017

Uh oh!

zmwangx commented Jan 25, 2017

Uh oh!

MikeMcQuaid commented Jan 26, 2017

Uh oh!

MikeMcQuaid commented Feb 12, 2017

Uh oh!

zmwangx commented Feb 13, 2017

Uh oh!

zmwangx commented Feb 13, 2017

Uh oh!

MikeMcQuaid commented Feb 13, 2017

Uh oh!

MikeMcQuaid commented Mar 18, 2017

Uh oh!

zmwangx commented Mar 18, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid commented Mar 21, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

zmwangx commented Dec 26, 2016 •

edited

Loading

zmwangx commented Jan 25, 2017 •

edited

Loading