Revert "login-sessions: Set the correct filename depending on default session type"
This reverts commit b93a0639.
-
Developer
Pardon me but where was this discussed? Or reviewed?
-
Developer
While there's a lack of communication on the commit, this is actually the correct thing to do:
$ git revert a6b28911869f00258 [check_build_failure 59eca83c8] Revert "Revert "login-sessions: Set the correct filename depending on default session type"" 2 files changed, 13 insertions(+), 4 deletions(-) $ cd ../../build/plasma-workspace $ make Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. -- Found KF5: success (found suitable version "5.78.0", minimum required is "5.77") found components: Plasma DocTools Runner Notifications NotifyConfig Su NewStuff Wallet IdleTime Declarative I18n KCMUtils TextWidgets KDELibs4Support Crash GlobalAccel DBusAddons Wayland CoreAddons People ActivitiesStats Activities KIO Prison Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. -- Found KF5: success (found version "5.78.0") found components: PlasmaQuick Installing in /home/tcanabrava/KDE/install. Run /data/Projects/KDE/build/plasma-workspace/prefix.sh to set the environment for plasma-workspace. -- Found KF5: success (found version "5.78.0") found components: Package -- Warning: Property DESCRIPTION for package Fontconfig already set to "Fontconfig is a library for configuring and customizing font access", overriding it with "Font access configuration library" -- Warning: Property URL already set to "https://www.fontconfig.org/", overriding it with "https://www.freedesktop.org/wiki/Software/fontconfig" -- Found Wayland: /usr/lib/libwayland-client.so (found version "1.18.0") found components: Client -- XCB: IMAGE requires XCB;SHM -- Found XCB: /usr/lib/libxcb.so;/usr/lib/libxcb-shm.so;/usr/lib/libxcb-image.so;/usr/lib/libxcb-randr.so (found version "1.14") found components: XCB RANDR IMAGE CMake Error at login-sessions/CMakeLists.txt:7 (set): set given invalid arguments for CACHE mode. CMake Error at login-sessions/CMakeLists.txt:8 (set): set given invalid arguments for CACHE mode. CMake Error at login-sessions/CMakeLists.txt:12 (install): install FILES given directory "/data/Projects/KDE/build/plasma-workspace/login-sessions/" to install. CMake Error at login-sessions/CMakeLists.txt:18 (install): install FILES given directory "/data/Projects/KDE/build/plasma-workspace/login-sessions/" to install. ... snip ... -- Configuring incomplete, errors occurred! See also "/data/Projects/KDE/build/plasma-workspace/CMakeFiles/CMakeOutput.log". See also "/data/Projects/KDE/build/plasma-workspace/CMakeFiles/CMakeError.log". make: *** [Makefile:5619: cmake_check_build_system] Error 1 $
So the real errors are: 1 - broken commit on master on the original commit 2 - revert without explanation of the reason
-
Developer
Where the problem with the original commit -- which wasn't caught in review and hasn't systematically broken builds, which is weird -- is this bit:
set(PLASMA_X11_DESKTOP_FILENAME plasmax11.desktop CACHE INTERNAL)
where the CMake documentation says that a docstring is mandatory after
CACHE
:set(<variable> <value>... CACHE <type> <docstring> [FORCE]) The ``<docstring>`` must be specified as a line of text providing a quick summary of the option for presentation to ``cmake-gui(1)`` users.
-
Author Developer
Hi, I apologize for this immediate revert, I worried about making things work correctly and ended up forgetting to explain the reason for the revert. Tomaz already had a conversation with me and guided me on how to proceed correctly in these cases, so I hope it didn't cause any problems.
-
Contributor
Or, you know, I can just submit a fixed version of the commit again?
-
Developer
That's obviously fine as well.
-
mentioned in merge request !532 (merged)
-
Contributor
MR with fixed commit: !532 (merged)
-
mentioned in commit neon/kde/plasma-workspace@b7eb0cf4