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

Unbreak qt5.full, add qt3d, qtgamepad, qtremoteobjects #102490

Closed
wants to merge 4 commits into from
Closed

Unbreak qt5.full, add qt3d, qtgamepad, qtremoteobjects #102490

wants to merge 4 commits into from
+45 −27

Conversation

Lucus16
Copy link
Contributor

@Lucus16 Lucus16 commented Nov 2, 2020

Motivation for this change

qtwebkit was already marked broken for qt 5.15 and the last commit I found is more than a year old so it is unlikely to be fixed. It broke qt5.full by being a part of it so I removed it.

For some qt modules, the default output is bin while the shared objects are still in out. I added out to the used outputs of the qt5.full environment so all shared objects are included. Maybe the default output of the qt modules should always be out instead?

Added more qt modules: qt3d, qtgamepad, qtremoteobjects

I tested the changes by building qt5{12,14,15}.full and running an application that depends on the added qt modules.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Sorry, something went wrong.

@Lucus16 Lucus16 requested a review from ttuegel as a code owner November 2, 2020 15:30
SuperSandro2000
SuperSandro2000 reviewed Nov 2, 2020
View reviewed changes
pkgs/development/libraries/qt-5/5.15/default.nix Outdated
@@ -141,19 +127,18 @@ let
qtwebchannel = callPackage ../modules/qtwebchannel.nix {};
qtwebengine = callPackage ../modules/qtwebengine.nix {};
qtwebglplugin = callPackage ../modules/qtwebglplugin.nix {};
qtwebkit = callPackage ../modules/qtwebkit.nix {};
Copy link
Member

Choose a reason for hiding this comment

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

Is this the eval error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

ofborg fails to eval everything and until that I can't run nixpkgs-review.

@Lucus16 Lucus16 force-pushed the qt branch 2 times, most recently from bb640fe to 7d8ad16 Compare November 6, 2020 09:17
@ofborg ofborg bot added 8.has: clean-up 8.has: package (new) This PR adds a new package labels Nov 6, 2020
@ofborg ofborg bot requested review from svanderburg and Phreedom November 6, 2020 09:44
@ofborg ofborg bot requested review from zraexy and ebzzry November 6, 2020 11:29
jonringer
jonringer reviewed Nov 6, 2020
View reviewed changes
pkgs/top-level/python-packages.nix Outdated
Comment on lines 5335 to 5339
pyqt5_with_qtwebkit =
pkgs.libsForQt514.callPackage ../development/python-modules/pyqt/5.x.nix {
pythonPackages = self;
withWebKit = true;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @FRidh

@veprbl veprbl mentioned this pull request Nov 12, 2020
Closed
69 tasks
@doronbehar
Copy link
Contributor

@Lucus16 some of the changes here look good. They would be worth putting in a separate PR or PRs, as now qt515.qtwebkit builds and works and the new qt modules added don't relate to qtwebkit for as far as I understand.

@SuperSandro2000 SuperSandro2000 added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 25, 2020
@SuperSandro2000
Copy link
Member

@Lucus16 Please resolve the merge conflict.

@Lucus16
qt5.full: Add all shared objects

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
5e3069a 
For some qt modules, the default output is bin while the shared objects
are still in out. This makes sure all shared objects show up in the
qt5.full env. Maybe the default output should always be out instead?
@Lucus16
qt5: Add qt3d, qtgamepad, qtremoteobjects

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
d1a08b3
@Lucus16
kdeApplications: remove qtwebkit dependency

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
a9953c0 
These packages don't seem to need qtwebkit anymore:

k3b
kreport
rocs
@Lucus16
calligra: remove overrides

Verified

This commit was signed with the committer’s verified signature.
vszakats Viktor Szakats
GPG key ID: B5ABD165E2AEF201
Verified
Learn about vigilant mode
2f31938
@Lucus16
Copy link
Contributor Author

Lucus16 commented Dec 26, 2020

Rebased. I don't have access to a build machine right now so I haven't tested.

@ofborg ofborg bot removed 6.topic: python 2.status: merge conflict This PR has merge conflicts with the target branch labels Dec 26, 2020
doronbehar
doronbehar reviewed Dec 27, 2020
View reviewed changes
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Just a few nits about the commit log and not the changes themselves. Otherwise looks great to me.

pkgs/development/libraries/qt-5/5.12/default.nix
Comment on lines -202 to +208
qtcharts qtconnectivity qtdeclarative qtdoc qtgamepad qtgraphicaleffects
qtimageformats qtlocation qtmultimedia qtquickcontrols qtquickcontrols2
qtscript qtsensors qtserialport qtsvg qttools qttranslations
qtvirtualkeyboard qtwebchannel qtwebengine qtwebkit qtwebsockets
qtwebview qtx11extras qtxmlpatterns
qt3d qtcharts qtconnectivity qtdeclarative qtdoc qtgamepad
qtgraphicaleffects qtimageformats qtlocation qtmultimedia
qtquickcontrols qtquickcontrols2 qtremoteobjects qtscript qtsensors
qtserialport qtsvg qttools qttranslations qtvirtualkeyboard qtwebchannel
qtwebengine qtwebkit qtwebsockets qtwebview qtx11extras qtxmlpatterns
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: It'd be nice if the diff was cleaner and would only include the added modules.

pkgs/applications/kde/rocs.nix
@@ -1,7 +1,7 @@
{
mkDerivation, lib,
extra-cmake-modules, boost,
qtbase, qtscript, qtquickcontrols, qtwebkit, qtxmlpatterns, grantlee,
qtbase, qtscript, qtquickcontrols, qtxmlpatterns, grantlee,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: The changes in kdeApplications: remove qtwebkit dependency IMO should be split to 3 - each change for each package.

pkgs/applications/office/calligra/default.nix
@@ -8,7 +8,7 @@
kwindowsystem, kxmlgui, sonnet, threadweaver,
kcontacts, akonadi, akonadi-calendar, akonadi-contacts,
eigen, git, gsl, ilmbase, kproperty, kreport, lcms2, marble, pcre, libgit2, libodfgen,
librevenge, libvisio, libwpd, libwpg, libwps, okular, openexr, openjpeg, phonon,
Copy link
Contributor

Choose a reason for hiding this comment

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

Another nit: The commit for calligra should explain that openjpeg is used / not used / isn't mandatory for it's build? And that the latest poppler can be used and poppler_0_61 is not needed.

@stale
Copy link

stale bot commented Jun 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 26, 2021
@SuperSandro2000
Copy link
Member

Closing due to inactivity from author.

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

@SuperSandro2000 SuperSandro2000

@jonringer jonringer

@doronbehar doronbehar

@ttuegel ttuegel

@svanderburg svanderburg

@Phreedom Phreedom

@FRidh FRidh

@zraexy zraexy

@ebzzry ebzzry

Assignees
No one assigned
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: qt/kde 8.has: clean-up 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@Lucus16 @doronbehar @SuperSandro2000 @jonringer