Skip to content

Comments

[opencv] Add opencv@3#35761

Closed
purcell wants to merge 3 commits intoHomebrew:masterfrom
purcell:opencv@3
Closed

[opencv] Add opencv@3#35761
purcell wants to merge 3 commits intoHomebrew:masterfrom
purcell:opencv@3

Conversation

@purcell
Copy link
Contributor

@purcell purcell commented Jan 7, 2019

OpenCV was recently bumped to 4.x in #35521, but a lot of non-Homebrew-packaged code does not yet compile against 4.x, which was only released just over a month ago.

Therefore, since there is an opencv@2 formula, it also seems prudent to provide opencv@3. This PR does that, based on the most recent version of opencv.rb for OpenCV 3.x.


  • Have you followed the guidelines for contributing?
  • Have you checked that there aren't other open pull requests for the same formula update/change?
  • Have you built your formula locally with brew install --build-from-source <formula>, where <formula> is the name of the formula you're submitting?
  • Is your test running fine brew test <formula>, where <formula> is the name of the formula you're submitting?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

@lembacon lembacon added new formula PR adds a new formula to Homebrew/homebrew-core legacy Relates to a versioned @ formula labels Jan 7, 2019
@SMillerDev
Copy link
Member

SMillerDev commented Jan 7, 2019

The tests fail on Jenkins, could you check those?

@SMillerDev SMillerDev added the test failure CI fails while running the test-do block label Jan 7, 2019
@purcell
Copy link
Contributor Author

purcell commented Jan 7, 2019

Thanks. I managed to reproduce this locally, and have pushed a fix: 79c7aef

I determined this by noting that the formula installed its Python modules into directories with the following names:

/usr/local/Cellar/opencv@3/3.4.5/lib/python2.7/ (6 files)
/usr/local/Cellar/opencv@3/3.4.5/lib/python3.7/ (6 files)

@fxcoudert
Copy link
Member

fxcoudert commented Jan 7, 2019

For what it's worth: the 3 formulas in Homebrew core that depend on OpenCV could be adapted to OpenCV 4 by a simple patch, which was already available upstream in most cases. The breaking changes appear limited.

The criteria for new versions formulas in Homebrew core are here https://docs.brew.sh/Versions. They include:

Upstream should have a release branch for the versioned formulae version and still make security updates for that version

So: does the OpenCV project maintain the 2.x branch, and plans to make future releases?

@fxcoudert fxcoudert removed the test failure CI fails while running the test-do block label Jan 7, 2019
url "https://github.com/opencv/opencv/archive/3.4.5.tar.gz"
sha256 "0c57d9dd6d30cbffe68a09b03f4bebe773ee44dc8ff5cd6eaeb7f4d5ef3b428e"

bottle do
Copy link
Member

Choose a reason for hiding this comment

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

No bottle block, please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No bottle block, please.

Removed in e1f9843

@purcell
Copy link
Contributor Author

purcell commented Jan 7, 2019

So: does the OpenCV project maintain the 3.x branch, and plans to make future releases?

Yes, according to this:

Branch 3.4 will be switched to maintanence mode: only bugfixes and light features will be accepted. BTW, release 3.4.4 is ready too!

@purcell
Copy link
Contributor Author

purcell commented Jan 7, 2019

For what it's worth: the 3 formulas in Homebrew core that depend on OpenCV could be adapted to OpenCV 4 by a simple patch, which was already available upstream in most cases. The breaking changes appear limited.

Agreed, but people also use Homebrew as a source of native libraries for non-Homebrew work, so bridging changes like this can be helpful. In my case, I'm using language bindings for OpenCV that are not yet compatible with OpenCV 4. With any luck, I'll get them updated, but it's still only a matter of weeks since the OpenCV 4 release. Obviously after a few months it would be best to drop these versioned formulae where possible: there may well be a case for dropping opencv@2 immediately.

@fxcoudert
Copy link
Member

@purcell our “problem” is: it's hard to drop a versioned formula once it's introduced, since people rely on it. While it's easier to just tell people affected negatively by the version migration to just pin the version on their local system for now, until their other software becomes compatible.

To give an example, last month opencv had 21,064 installs, and opencv@2 had 1,641. While the later is clearly smaller, it's not exactly negligible.

@purcell
Copy link
Contributor Author

purcell commented Jan 7, 2019

Yep, understandably difficult to strike the right balance. You folks seem to do a wonderful job, though, so thanks!

@neggert
Copy link

neggert commented Jan 16, 2019

FWIW, there's at least one place where the 4.x release does not maintain feature parity with 3.x. Quoting from the changelog:

For now base64 support is not complete (only loading base64-encoded XML and YAML is supported, encoding is not supported at all)

I'm not going to be able to migrate my code until they fix this, so I'd really appreciate having opencv@3 available.

@fxcoudert
Copy link
Member

@BrewTestBot test this please

@fxcoudert fxcoudert added the ready to merge PR can be merged once CI is green label Jan 17, 2019
@fxcoudert
Copy link
Member

That all sounds reasonable, let's go for it!

@fxcoudert
Copy link
Member

Thanks @purcell for your contribution to Homebrew!

@fxcoudert fxcoudert closed this in 37344e0 Jan 18, 2019
@purcell
Copy link
Contributor Author

purcell commented Jan 21, 2019

Thanks @purcell for your contribution to Homebrew!

Cheers @fxcoudert, your help and work on homebrew is appreciated.

niheaven pushed a commit to niheaven/homebrew-core that referenced this pull request Jan 23, 2019
Closes Homebrew#35761.

Signed-off-by: FX Coudert <fxcoudert@gmail.com>
@lock lock bot added the outdated PR was locked due to age label Feb 20, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Feb 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

legacy Relates to a versioned @ formula new formula PR adds a new formula to Homebrew/homebrew-core outdated PR was locked due to age ready to merge PR can be merged once CI is green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants