Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 688366 - [kde overlay] >=kde-plasma/plasma-workspace-5.16.80: Restore Gentoo FHS script support
Summary: [kde overlay] >=kde-plasma/plasma-workspace-5.16.80: Restore Gentoo FHS scrip...
Status: CONFIRMED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Gentoo KDE team
URL:
Whiteboard:
Keywords: NeedPatch
Depends on:
Blocks:
 
Reported: 2019-06-19 19:03 UTC by Eugene Shalygin
Modified: 2019-07-16 18:38 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Eugene Shalygin 2019-06-19 19:03:43 UTC
* Applying plasma-workspace-5.14.80-startkde-script.patch ...
The text leading up to this was:
--------------------------
|From bf569560bf195ac4e79d65d4103a7161a6a2f2ac Mon Sep 17 00:00:00 2001
|From: Elias Probst <mail@eliasprobst.eu>
|Date: Sat, 4 Jul 2015 11:33:01 +0200
|Subject: [PATCH] [startkde] Gentoo FHS script support
|
|---
| startkde/startkde.cmake | 10 ++++++++++
| 1 file changed, 10 insertions(+)
|
|diff --git a/startkde/startkde.cmake b/startkde/startkde.cmake
|index 2585600..9350158 100644
|--- a/startkde/startkde.cmake
|+++ b/startkde/startkde.cmake
--------------------------
No file to patch.  Skipping patch.
2 out of 2 hunks ignored
Comment 1 Andreas Sturmlechner gentoo-dev 2019-06-20 10:23:29 UTC
Elias, are you up to speed with how this patch should be changed after the recent upstream changes?
Comment 2 Elias Probst 2019-06-20 14:18:43 UTC
Sorry, I'm out of the loop wrt Gentoo/KDE since more than a year - can't really help here.
Comment 3 Duncan 2019-06-23 05:05:50 UTC
I was about to file a bug on this myself.
Comment 4 Duncan 2019-06-23 05:14:48 UTC
(In reply to Andreas Sturmlechner from comment #1)
> Elias, are you up to speed with how this patch should be changed after the
> recent upstream changes?

Not Elias and I'm definitely more comfortable in shell than C++, but...

You're referring to upstream commit 33ff53ca2 (and followups), where they're replacing the startkde shell-script startup with a native-compiled executable, leaving us no shell-script to patch, correct?

ATM I'm following sourceFile() in the cpp files and the plasma-sourceenv.sh helper-script.  That seems to be the functionality we need, as ported across.  (I traced it in the initial commit and am working on the followup commits that refactor things now.)
Comment 5 Duncan 2019-06-23 06:56:10 UTC
(In reply to Duncan from comment #4)
> 
> ATM I'm following sourceFile() in the cpp files and the plasma-sourceenv.sh
> helper-script.  That seems to be the functionality we need, as ported
> across.  (I traced it in the initial commit and am working on the followup
> commits that refactor things now.)

[Pardon me if I'm way too detailed for my audience here, but if it saves the implementer even part of the several hours I spent looking into it...]

OK, as of 71a03a0b7 (current HEAD), ...

Looks like the code we're after is in startkde/startplasma.cpp, which is included in the final binaries that start both x11 and wayland.

As mentioned above, the core functionality we need is sourceFile(), which calls plasma-sourceenv.sh as a helper-script.

The caller we're interested in is runEnvironmentScripts() which has no parameters  and (sort-of) of hard-coding the config directories it sources from.  It is in turn called from main() in both startplasma-x11.cpp and startplasma-wayland.cpp.


Since runEnvironmentScripts() hard-codes its directory (well, sort-of, it gets it from the config) we can't use it directly, but we can use it as a pattern for our own code to call sourceFile().

I'd probably create a single runEnvironmentScriptsGentoo() in startplasma.cpp that takes a parameter pointing at the appropriate dir, and add one-line calls to it from startplasma-x11.cpp and startplasma-wayland.cpp (so they're directly mirroring the startup calls to the the original runEnvironmentScript() in those two files, tho of course that means adding the gentoo-suffixed function to the header as well.

It appears there's no native shutdown functionality (at this level anyway) to parallel, so as with the old patch we'll be adding our own shutdown-script calls to both startplasma-wayland.cpp and startplasma-x11.cpp, probably right before the cleanupPlasmaEnvironment() call in each, as that's most directly parallel to the old scripts placement.  Again, call runEnvironmentScriptsGentoo() in startplasma.cpp with a single passed parameter, making the call to it in the separate x11 and wayland files a single line.

Better yet, ask upstream to make runEnvironmentScript take a directory parameter so we don't have to reimplement it, and all we'd have to do would be add the four single-line calls to it, one each for startup and shutdown, in each of the x11 and wayland cpp files.  That way we'd get future code updates to it without having to mirror them to our own patch as well.  =:^)  (Tho that'll probably take some time to get approved, and the approach above can be taken until the directory-parametered version is upstreamed.)
Comment 6 Duncan 2019-06-23 10:27:33 UTC
Another note, for anyone else just trying to get live-git plasma-workspace-9999 updating and building again, with or without the gentoo customization added by that patch, lest some other bug trigger and get hard to trace due to too long between updates on the live build.

Since I don't use the functionality at issue (nothing but the gentoo-default agent scripts in the dirs in question and I don't use them), I decided to simply try killing the patch[1].

That wasn't enough.  I had to kill the one for wayland too, since that script is replaced by a native executable too.

But that wasn't enough either.  There's a sed call in src_prepare that fails due to missing those same two scripts now.

Fortunately it's a four-line sed script killing the four lines in the ebuild, so it's easy enough to continue applying to further updates and/or to kill when this bug is fixed.

Anyway, that got plasma-workspace building again and it installed fine, but then X would start but quit after a few seconds without starting kde/plasma.

~/.xsessionerrors revealed the problem.  My (custom) /etc/X11/Xsessions was failing, as it had "exec startkde", and the new setup replaces the old startkde script with a native startplasma-x11 executable.  After replacing that with "exec startplasma-x11", I could start kde from my terminal login once again.

So while that post-install problem isn't gentoo's, there's more to fixing the ebuild than killing or updating that first patch.  It's two patches and removal of that sed.  Plus potentially updating the kde/plasma session launch scripts if they call startkde to call startplasma-x11 instead now. I'm not sure on the last because as I said I have a custom script, which /did/ need updated.

---
[1] I've an update script that applies ebuild patches or seds automatically, much as portage does with /etc/portage/patches, so once I figure out what I want to change in the ebuild, it's trivial to add a patch or sed script to the appropriate dir so it gets auto-applied after that.
Comment 7 Andreas Sturmlechner gentoo-dev 2019-07-02 13:53:56 UTC
Is there any specific reason why we can't simply use these?

$HOME/.config/autostart
$HOME/.config/plasma-workspace/shutdown
Comment 8 Duncan 2019-07-03 11:56:23 UTC
(In reply to Andreas Sturmlechner from comment #7)
> Is there any specific reason why we can't simply use these?
> 
> $HOME/.config/autostart
> $HOME/.config/plasma-workspace/shutdown

Those are individual user dirs, not the system-wide locations that package-management can legitimately touch.

Which was if I'm reading the comment in the patch correctly, the point of the patch.  There are system-parallel XDG locations matching the user locations, but the startup/shutdown scripts weren't looking in them, only the user locations.

Of course we should try upstreaming the patch as well once we've come up with the new version, thus getting rid of our maintenance headaches by upstreaming them, but we gotta create a patch, first.
Comment 9 jospezial 2019-07-03 22:32:52 UTC
(In reply to Duncan from comment #3)
> I was about to file a bug on this myself.

confirming
Comment 10 jospezial 2019-07-03 23:25:38 UTC
If I disable that patch it fails to apply plasma-workspace-5.10-startplasmacompositor-script.patch.

If I disable that, then sed fails:

>>> Preparing source in /var/tmp/portage/kde-plasma/plasma-workspace-9999/work/plasma-workspace-9999 ...
 * Applying plasma-workspace-5.14.2-split-libkworkspace.patch ...                                                                                                     [ ok ]
sed: can't read startkde/startkde.cmake: No such file or directory
sed: can't read startkde/startplasmacompositor.cmake: No such file or directory
Comment 11 Andreas Sturmlechner gentoo-dev 2019-07-04 17:33:24 UTC
We are aware of that already.
Comment 12 Larry the Git Cow gentoo-dev 2019-07-16 18:37:13 UTC
The bug has been referenced in the following commit(s):

https://gitweb.gentoo.org/proj/kde.git/commit/?id=3f668bb654c817ec2b40cca7c2426c579d336bd8

commit 3f668bb654c817ec2b40cca7c2426c579d336bd8
Author:     Andreas Sturmlechner <asturm@gentoo.org>
AuthorDate: 2019-07-16 18:36:48 +0000
Commit:     Andreas Sturmlechner <asturm@gentoo.org>
CommitDate: 2019-07-16 18:36:48 +0000

    kde-plasma/plasma-workspace: Drop broken patches for now
    
    Bug: https://bugs.gentoo.org/688366
    Package-Manager: Portage-2.3.69, Repoman-2.3.16
    Signed-off-by: Andreas Sturmlechner <asturm@gentoo.org>

 kde-plasma/plasma-workspace/plasma-workspace-9999.ebuild | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)