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

Add KDE Frameworks Tier 1 #2495

Closed
wants to merge 1 commit into from

Conversation

3 participants
@PureTryOut
Copy link
Contributor

commented Oct 14, 2017

Adds tier 1 packages of the KDE frameworks. Originally packaged for postmarketOS.

Only Kirigami2 is missing due to CMake not being able to find the header files of qt5-qtquickcontrols2. However, qt5-qtquickcontrols2 does not seem to have header files as adding the $pkgname-dev subpackage fails.

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch from 288bc7c to b059db7 Oct 14, 2017

@kaniini

This comment has been minimized.

Copy link
Contributor

commented Oct 14, 2017

@awilfox what is your opinion on this vs the KDE packaging in adelie?

@awilfox

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2017

  • I don't understand why a lot of these packages depend on clang or py3-qt; they don't use either.
  • There are definitely no packages that require Python 2, but a few have them in dependencies.
  • No KF5 package has a direct requirement on Mesa. There are a few in the Applications collection that require GLU, but I haven't seen raw GLX calls anywhere in KF5.
  • Tests are disabled on a large swath of these despite having perfectly valid tests. A few of the packages do require X11 running, and I've noted that in the Adélie builds in the hopes of extending abuild's check support to include virtual X11 support. Plenty of the frameworks do not use X11 in their tests, however.
  • The patching is unnecessarily complex. If the patches were written correctly and the packages used $builddir instead of hardcoding $srcdir/build (where KDE packages, with the exception of kactivities, do not even require out-of-source building) then they could use default_prepare and not even have prepare phases.
  • All of the kinit/etc patches are not required as long as extra-cmake-modules is patched to provide _XOPEN_SOURCE which is what we do. This takes care of packages in the Applications collection as well as the Plasma collection.
  • None of these packages declare $depends_dev, so they wouldn't be very usable to anyone who wants to pull them in alone (LXQt uses KWindowSystem and is being ported to KGlobalAccel), and it causes the makedepends of each one to be bloated.

I can do a more thorough review later if desired.

I do however like the patch to syntax-highlighting that adds APKBUILD support. That's pretty nifty.

@awilfox

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2017

Additionally:

  • There are packages that build QCH sets that require GraphViz (/usr/bin/dot) but do not have GraphViz in their makedepends. This will cause the build to fail.
  • kscreenlocker is the only package that declares a dependency of linux-pam but plasma-workspace is what provides the /etc/pam.d/kde file, which seems odd to me. Plasma Workspace doesn't even use PAM; only kscreenlocker does, and that is the package that should have that file installed. (You can use kscreenlocker from other desktops, if you wanted to, and it wouldn't work without that.)
  • Speaking of kscreenlocker, it requires SUID to read /etc/shadow, otherwise sessions will not be unlockable.
  • Plasma Workspace is missing a lot of shell runtime deps. It is also unstable 5.11 version which has known bugs with Wayland.
  • KWin does not require ConsoleKit; it is an optional, though definitely recommended, dependency of Plasma itself. Plasma can use ConsoleKit for seat management and it can also be used to unlock the session if the session unlocker fails (I've seen that happen a few times due to ENFILE.), but KWin itself doesn't.
  • Sonnet has no backends enabled. This makes it entirely useless. (In Adélie we split aspell and build hunspell in by default, since that is what upstream recommends.)

Should I just open a PR with our KF5 packaging? I'm in the process of upgrading it from 5.38.0 to 5.39.0, and then I wouldn't have a problem with it if Alpine is ready/willing to ship them.

(Personally, I was waiting until Plasma Desktop was more heavily tested, and then we were going to offer the entire Plasma environment as one PR. If KF5 would be useful to Alpine beyond Plasma, we can definitely provide that.)

@awilfox

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2017

I've just noticed one more pretty fatal flaw: start_kdeinit has its SUID bit removed in the testing/kinit package due to "SUID violating Alpine standards" (options="suid" is what to use when it is needed). KInit can be built with libcap to avoid SUID, and that's what we do in Adélie; without one of these, the kdeinit process becomes vulnerable to the Linux OOM killer.

@awilfox

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2017

One idea I've just had is to commit all tier 1 frameworks first (as they are required to not have any dependencies on any KF5 libs). That way they can all be built and Travis won't throw errors due to the other deps missing.

Then commit tier 2 frameworks which can only depend on tier 1, then tier 3, then Plasma which has a few tier 4 deps. This would be four PRs, but they would be verifiable by Travis, and ensure issues are caught quickly and can therefore be resolved quickly.

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch from b059db7 to f83d7f9 Oct 15, 2017

@PureTryOut PureTryOut changed the title KDE Frameworks and Plasma Add KDE Frameworks Tier 1 Oct 15, 2017

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch 4 times, most recently from a168321 to 7d2b6d8 Oct 15, 2017

@PureTryOut

This comment has been minimized.

Copy link
Contributor Author

commented Oct 15, 2017

I've fixed most things mentioned. Travis reports build failed due to hitting the time limit, not because a compilation failed.

I'll add the packages from the other tiers in separate PR's.

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch 2 times, most recently from 7116348 to 7c6504f Oct 17, 2017

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch 2 times, most recently from 9013336 to bdb861a Oct 25, 2017

@kaniini
Copy link
Contributor

left a comment

I wish to reiterate that I prefer to use LTS versions of KDE Frameworks tier 1/2/3.

Plasma Mobile next can be built against the LTS versions.

@@ -0,0 +1,35 @@
# Contributor: Bart Ribbers <bribbers@disroot.org>
# Maintainer: Bart Ribbers <bribbers@disroot.org>
pkgname=attica-qt5

This comment has been minimized.

Copy link
@kaniini

kaniini Oct 27, 2017

Contributor

Why does this package name divert from upstream package name?

Do you honestly believe it will be built against Qt4?

@@ -0,0 +1,35 @@
# Contributor: Bart Ribbers <bribbers@disroot.org>
# Maintainer: Bart Ribbers <bribbers@disroot.org>
pkgname=polkit-qt5

This comment has been minimized.

Copy link
@kaniini

kaniini Oct 27, 2017

Contributor

Again, this should be polkit-qt, to match upstream. It won't ever be built against Qt4.

-DCMAKE_INSTALL_PREFIX=/usr \
-DKDE_INSTALL_LIBDIR=lib \
-DBUILD_QCH=ON
make

This comment has been minimized.

Copy link
@kaniini

kaniini Oct 27, 2017

Contributor

indention level is wrong

@PureTryOut PureTryOut force-pushed the PureTryOut:kde branch from bdb861a to 2e84d81 Oct 27, 2017

@PureTryOut

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2017

Could you guys as the Alpine team have some internal discussion about this? On the mailing list someone mentioned that he preferred non-LTS instead.

@PureTryOut

This comment has been minimized.

Copy link
Contributor Author

commented Oct 27, 2017

Since this PR only seems to cause trouble, which we obviously do not want, I'll close this and let Adélie handle upstreaming of KDE stuff.

@PureTryOut PureTryOut closed this Oct 27, 2017

@PureTryOut PureTryOut deleted the PureTryOut:kde branch Oct 27, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.