plasma5.plasma-workspace: fix patch #74830

Kiwi · 2019-12-02T12:39:55Z

At some point a patch accidentally removed

done
break

from an if then/while loop in startkde.

Which still breaks, but not the way we want...

plasma-workspace-5.16.5/bin/startkde: line 403: syntax error near unexpected token `fi'
plasma-workspace-5.16.5/bin/startkde: line 403: `        fi'

Motivation for this change

I've been having more problems with KDE lately than normal and while I was going through my journalctl I came across the error message. I'm not sure if it's related to my logout problems or not but... hopefully.

unpackPhase had this source (startkde.cmake)

if test x"$wait_drkonqi"x = x"true"x ; then
    # wait for remaining drkonqi instances with timeout (in seconds)
    wait_drkonqi_timeout=`kreadconfig5 --file startkderc --group WaitForDrKonqi --key Timeout --default 900`
    wait_drkonqi_counter=0
    while qdbus | grep "^[^w]*org.kde.drkonqi" > /dev/null ; do
        sleep 5
        wait_drkonqi_counter=$((wait_drkonqi_counter+5))
        if test "$wait_drkonqi_counter" -ge "$wait_drkonqi_timeout" ; then
            # ask remaining drkonqis to die in a graceful way
            qdbus | grep 'org.kde.drkonqi-' | while read address ; do
                qdbus "$address" "/MainApplication" "quit"
            done
            break
        fi
    done
fi

After the patchPhase

if [ x"$wait_drkonqi"x = x"true"x ]; then
    # wait for remaining drkonqi instances with timeout (in seconds)
    wait_drkonqi_timeout=$(@NIXPKGS_KREADCONFIG5@ --file startkderc --group WaitForDrKonqi --key Timeout --default 900)
    wait_drkonqi_counter=0
    while @NIXPKGS_QDBUS@ | @NIXPKGS_GREP@ -q "^[^w]*org.kde.drkonqi" ; do
        sleep 5
        wait_drkonqi_counter=$((wait_drkonqi_counter+5))
        if [ "$wait_drkonqi_counter" -ge "$wait_drkonqi_timeout" ]; then
            # ask remaining drkonqis to die in a graceful way
            @NIXPKGS_QDBUS@ | @NIXPKGS_GREP@ 'org.kde.drkonqi-' | while read address ; do
                @NIXPKGS_QDBUS@ "$address" "/MainApplication" "quit"
        fi
    done
fi

What ends up in bin/startkde

if [ x"$wait_drkonqi"x = x"true"x ]; then
    # wait for remaining drkonqi instances with timeout (in seconds)
    wait_drkonqi_timeout=$(/nix/store/s9a9fv0nkq1zaqxcs6p19fg4d6p6nrv2-kconfig-5.62.0-bin/bin/kreadconfig5 --file startkderc --group WaitForDrKonqi --key Timeout --default 900)
    wait_drkonqi_counter=0
    while /nix/store/8cimdfy053rw5lgsm570c4m0rffb4g4g-qttools-5.12.5-bin/bin/qdbus | /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep -q "^[^w]*org.kde.drkonqi" ; do
        sleep 5
        wait_drkonqi_counter=$((wait_drkonqi_counter+5))
        if [ "$wait_drkonqi_counter" -ge "$wait_drkonqi_timeout" ]; then
            # ask remaining drkonqis to die in a graceful way
            /nix/store/8cimdfy053rw5lgsm570c4m0rffb4g4g-qttools-5.12.5-bin/bin/qdbus | /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep 'org.kde.drkonqi-' | while read address ; do
                /nix/store/8cimdfy053rw5lgsm570c4m0rffb4g4g-qttools-5.12.5-bin/bin/qdbus "$address" "/MainApplication" "quit"
        fi
    done
fi

What shellcheck had to say about that (in red, really angry!)

In /run/current-system/sw/bin/startkde line 399:
        if [ "$wait_drkonqi_counter" -ge "$wait_drkonqi_timeout" ]; then
                                                                    ^-- SC1009: The mentioned syntax error was in this then clause.


In /run/current-system/sw/bin/startkde line 401:
            /nix/store/8cimdfy053rw5lgsm570c4m0rffb4g4g-qttools-5.12.5-bin/bin/qdbus | /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep 'org.kde.drkonqi-' | while read address ; do
                                                                                                                                                                             ^-- SC1073: Couldn't parse this while loop. Fix to allow more checks.
                                                                                                                                                                                                  ^-- SC1061: Couldn't find 'done' for this 'do'.


In /run/current-system/sw/bin/startkde line 403:
        fi
        ^-- SC1062: Expected 'done' matching previously mentioned 'do'.
          ^-- SC1072: Unexpected keyword/token. Fix any mentioned problems and try again.

For more information:
  https://www.shellcheck.net/wiki/SC1061 -- Couldn't find 'done' for this 'do'.
  https://www.shellcheck.net/wiki/SC1062 -- Expected 'done' matching previous...
  https://www.shellcheck.net/wiki/SC1072 -- Unexpected keyword/token. Fix any...

And now, what you've all been waiting for....!

What it looks like with this patch

if [ x"$wait_drkonqi"x = x"true"x ]; then
    # wait for remaining drkonqi instances with timeout (in seconds)
    wait_drkonqi_timeout=$(/nix/store/8h096wmzmxpfz25bxsxm3zyrpaf1iw90-kconfig-5.62.0-bin/bin/kreadconfig5 --file startkderc --group WaitForDrKonqi --key Timeout --default 900)
    wait_drkonqi_counter=0
    while /nix/store/bd56x7j9cas7pg37cm2b9gvzv16qbd3h-qttools-5.12.6-bin/bin/qdbus | /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep -q "^[^w]*org.kde.drkonqi" ; do
        sleep 5
        wait_drkonqi_counter=$((wait_drkonqi_counter+5))
        if [ "$wait_drkonqi_counter" -ge "$wait_drkonqi_timeout" ]; then
            # ask remaining drkonqis to die in a graceful way
            /nix/store/bd56x7j9cas7pg37cm2b9gvzv16qbd3h-qttools-5.12.6-bin/bin/qdbus | /nix/store/20b535jb98hy7k4z8vkrlkjma212a3l5-gnugrep-3.3/bin/grep 'org.kde.drkonqi-' | while read address ; do
                /nix/store/bd56x7j9cas7pg37cm2b9gvzv16qbd3h-qttools-5.12.6-bin/bin/qdbus "$address" "/MainApplication" "quit"
            done
            break
        fi
    done
fi

Which makes shellcheck a bit less angry about that and start mentioning the rest of the file should use globbing and that read without -r mangles backslashes. Whatever that means.

Things done

I spent way too long (hours, if not days) trying to figure out how all of this worked... and all just so I could do what ultimately amounts to nothing more than adding two lines to a bash script. Well, not removing two lines from a cmake file. :)

~/projects/nixpkgs tracks NixOS/nixpkgs master, and pr-nixpkgs has this PR... (after unpackPhase and patchPhase)

[nix-shell:~/projects/nixpkgs]$ diff -ru plasma-workspace-5.16.5/ ~/projects/github/pr-nixpkgs/plasma-workspace-5.16.5/

diff -ru plasma-workspace-5.16.5/startkde/startkde.cmake /home/kiwi/projects/github/pr-nixpkgs/plasma-workspace-5.16.5/startkde/startkde.cmake
--- plasma-workspace-5.16.5/startkde/startkde.cmake	2019-12-02 10:33:10.319036592 +0000
+++ /home/kiwi/projects/github/pr-nixpkgs/plasma-workspace-5.16.5/startkde/startkde.cmake	2019-12-02 08:37:56.851321636 +0000
@@ -400,6 +400,8 @@
             # ask remaining drkonqis to die in a graceful way
             @NIXPKGS_QDBUS@ | @NIXPKGS_GREP@ 'org.kde.drkonqi-' | while read address ; do
                 @NIXPKGS_QDBUS@ "$address" "/MainApplication" "quit"
+            done
+            break
         fi
     done
 fi

I don't know that this change would matter; but plasma5 tests still passed.

Notify maintainers

cc @

At some point a patch accidentally removed ``` done break ``` from an if then/while loop in `startkde`. Which still breaks, but not the way we want... plasma-workspace-5.16.5/bin/startkde: line 403: syntax error near unexpected token `fi' plasma-workspace-5.16.5/bin/startkde: line 403: ` fi'

ttuegel

Wow, well spotted! I think the patch must have gotten mangled during an earlier version bump. Thanks for fixing this! 😃

worldofpeace · 2019-12-02T18:08:28Z

Might be worthwhile having an Updating Plasma5 docs in nixpkgs.
Typically with these patches you need to be careful on regenerating them/verfiying them and hopefully everyone does a similar procedure.

Kiwi · 2019-12-10T08:50:55Z

Might be worthwhile having an Updating Plasma5 docs in nixpkgs.
Typically with these patches you need to be careful on regenerating them/verfiying them and hopefully everyone does a similar procedure.

There was a comment in (at least) one of the files that could definitely have been more helpful. I'm not sure if it's entirely inaccurate but it seemed out of date at best and in my case outright misleading. :( The lack of "ensured relevant documentation was up to date" was intentional... I could have probably done a bit more there, but eh, handwaves reasons. >.>

Kiwi requested a review from ttuegel as a code owner December 2, 2019 12:39

ofborg bot added 6.topic: qt/kde 10.rebuild-darwin: 0 10.rebuild-linux: 11-100 labels Dec 2, 2019

ttuegel approved these changes Dec 2, 2019

View reviewed changes

worldofpeace approved these changes Dec 2, 2019

View reviewed changes

worldofpeace requested a review from dtzWill December 2, 2019 18:09

ttuegel merged commit f35a715 into NixOS:master Dec 8, 2019

Kiwi deleted the startkde-broken branch January 10, 2020 05:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plasma5.plasma-workspace: fix patch #74830

plasma5.plasma-workspace: fix patch #74830

Kiwi commented Dec 2, 2019

ttuegel left a comment

worldofpeace commented Dec 2, 2019

Kiwi commented Dec 10, 2019

plasma5.plasma-workspace: fix patch #74830

plasma5.plasma-workspace: fix patch #74830

Conversation

Kiwi commented Dec 2, 2019

Motivation for this change

Things done

Notify maintainers

ttuegel left a comment

Choose a reason for hiding this comment

worldofpeace commented Dec 2, 2019

Kiwi commented Dec 10, 2019