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.

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

qt5 5.7.1 and delete qt4 #8306

Closed
wants to merge 2 commits into from
Closed

qt5 5.7.1 and delete qt4 #8306

wants to merge 2 commits into from

Conversation

DomT4
Copy link
Member

@DomT4 DomT4 commented Dec 30, 2016

  • 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?
  • Does your build pass brew audit --strict <formula> (after doing brew install <formula>)?

==> Summary
🍺  /usr/local/Cellar/qt5/5.7.1: 8,273 files, 242.3M, built in 83 minutes 35 seconds

It's that time again.

Fixes #1705.

@MikeMcQuaid
Copy link
Member

Hurray for patches merged upstream! I think I may use this release as the one in which we boneyard Qt4. @DomT4 mind giving me commit access to this PR and I'll commit to it to combine with https://github.com/Homebrew/homebrew-core/pull/7019/files?

@DomT4
Copy link
Member Author

DomT4 commented Dec 30, 2016

Go for it; I've ticked the necessary box now.

@MikeMcQuaid
Copy link
Member

@DomT4 Done, thanks pal. ALMOST THERE!!!!

@DomT4
Copy link
Member Author

DomT4 commented Dec 30, 2016

Well, in 2:30hrs, providing nothing blows up on CI. Qt has taught me to never be an optimist around these PRs 😅.

@dakcarto
Copy link
Contributor

Hi,

Would you like me to take a crack at adding custom -plugindir to the mix for this release, or wait a bit? Given that this will replace the default qt, might be a good time to introduce it.

@MikeMcQuaid
Copy link
Member

@dakcarto Yes but don't want that to block this PR.

@MikeMcQuaid MikeMcQuaid changed the title qt5 5.7.1 qt5 5.7.1 and boneyard qt4 Dec 30, 2016
@MikeMcQuaid MikeMcQuaid changed the title qt5 5.7.1 and boneyard qt4 qt5 5.7.1 and delete qt4 Dec 30, 2016
@MikeMcQuaid
Copy link
Member

Made some fixes locally to not put this in the boneyard so we can use the qt alias and consider renaming qt5 to qt at some point in the future.

@MikeMcQuaid
Copy link
Member

Thanks for this @DomT4

@DomT4 DomT4 deleted the qt5lol branch December 30, 2016 21:23
@DomT4
Copy link
Member Author

DomT4 commented Dec 30, 2016

Thanks @MikeMcQuaid. Shall see how much pushback the project gets from Qt4 going away; there shouldn't be much by this point given all the issues Qt4 has but you never know sigh.

There's a second branch on homebrew-core at the moment, has been for a couple weeks, don't know if intentional? 😅.

@RandomDSdevel
Copy link
Contributor

@MikeMcQuaid: Just a (relatively) quick comment here, but having had both qt and qt5 installed prior to the effort that this PR is part of has made brew outdated report to me post-update that I have two outdated copies of qt5 installed! I'm correct in assuming that this is due to the fact that qt is now an alias to qt5, am I not? How would I resolve this down here on my end? It'd probably be best if I filed a new issue for this, but I thought I'd better mention my problem here before I did that just to give everybody involved a heads-up as to what I'm experiencing here.

@RandomDSdevel
Copy link
Contributor

@MikeMcQuaid: As reported by GitHub's notification of #8342's existence and mention of this PR in said issue, said issue is now ready and we can continue discussion of my problem there.

@scpeters
Copy link
Member

scpeters commented Dec 31, 2016

@DomT4 I have a tap that still uses qt4 (osrf/simulation), so this is an inconvenience, but it has been anticipated. If qt4 doesn't go to the boneyard, we'll probably fork the formula into our tap for use on yosemite and el_capitan while our users transition to the latest version of our software.

EDIT: I was just informed about the cartr/qt4 tap, so I will investigate that.

@dakcarto
Copy link
Contributor

Hi @scpeters,

We needed to do this for the QGIS 2-based formulae in the OSGeo4Mac tap, along with other Qt4 associated formulae as well. Our setup, unlike cartr/qt4, is isolated and does not conflict with any Homebrew changes. That repo was consulted during the work.

See 'qt-4' and formulae ending with '-qt4'. Note, this is an interim solution that will go away within 6-8 months, or whenever QGIS 3 becomes stable.

@MikeMcQuaid
Copy link
Member

@scpeters @cartr @dakcarto I strongly recommend you disable webkit in your Qt4 builds as it's extremely outdated at this point and otherwise you are making your users vulnerable. Obviously it's up to you but I felt it necessary to state that point.

@DomT4
Copy link
Member Author

DomT4 commented Dec 31, 2016

@scpeters Worth noting that Mike added the qt4 related commits, I just pushed the (for Qt5 remarkably simple) bump. I agree with the changes but didn't make and can't influence the future direction of them.

As he notes though, please try to disable WebKit if you have revived copies of qt4 in taps; it's a security mess even when regularly updated, and qt4 hasn't done regular updates for quite some time 🤷‍♂️.

@dakcarto
Copy link
Contributor

@MikeMcQuaid wrote

I strongly recommend you disable webkit in your Qt4 builds as it's extremely outdated at this point and otherwise you are making your users vulnerable. Obviously it's up to you but I felt it necessary to state that point.

Agreed. I'll look at making it optional. Unfortunately, without it, QGIS 2 builds are rendered fairly useless in many regards.

@scpeters
Copy link
Member

scpeters commented Jan 2, 2017

Thanks for the feedback @dakcarto and @MikeMcQuaid

I'm currently experimenting with a qt4-no-webkit formula for my tap to address the security concerns.

@chohner chohner mentioned this pull request Jan 2, 2017
2 tasks
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants