| Summary: | ~/.xsession ignored when xdg-compliance is installed | ||
|---|---|---|---|
| Product: | Mageia | Reporter: | Jan Smout <smout.jan> |
| Component: | RPM Packages | Assignee: | All Packagers <pkg-bugs> |
| Status: | RESOLVED OLD | QA Contact: | |
| Severity: | normal | ||
| Priority: | Normal | CC: | luigiwalser, marja11 |
| Version: | 5 | ||
| Target Milestone: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Source RPM: | xdg-compliance | CVE: | |
| Status comment: | |||
|
Description
Jan Smout
2016-04-12 10:38:45 CEST
Assigning to all packagers collectively, since there is no maintainer for xdg-compliance CC:
(none) =>
marja11 This sounds invalid. Something changed to fix valid bugs with normal ways of starting existing desktop environments broke some strange, local, custom way of starting KDE that's unsupported. If you're already running your own script to start KDE, you can have it run ~/.xsession if it isn't already. (In reply to David Walser from comment #2) > This sounds invalid. Something changed to fix valid bugs with normal ways > of starting existing desktop environments broke some strange, local, custom > way of starting KDE that's unsupported. If you're already running your own > script to start KDE, you can have it run ~/.xsession if it isn't already. This bug report is not about starting KDE in a custom way. And the problem at hand is not that I am not able to find any solution to this (I know how to remove packages or to possibly adapt kdmrc to run a custom Xsession if necessary). This bug report is about the ~/.xsession method being broken in M5/Cauldron. It is a very common use case and therefore valid. The $DESKTOP variable is specified in /etc/sysconfig/desktop. You don't expect the system to silently change it behind your back because some other tool needs *temporary* access to another value. The keyword here is temporary because most systems don't have icewm installed. Can you elaborate on the necessity of exporting DESKTOP=$SESSION? Which software needs it? Icewm? xdg_menu? Can it be made temporary? A patch could be made to the Xsession script to force keep the old DESKTOP variable, but that looks like patching one software to hide a bug in another. The issue was that /etc/X11/xinit.d/xdg-autostart was looking for $DESKTOP which was sometimes not set (like running startx instead of logging in via a DM), making xdg-autostart fail to run when it needed to. OK I've changed it to use the $SESSION variable instead. Please verify that it fixes your issue and also does not cause IceWM to fail to start mate-polkit, including when started via startx. The ~/.xsession thing is not a common use case, and is just there as a fall-through, but let's see if we can make it work again. Note that *all* Mageia systems generally have icewm installed unless you specifically uninstalled it. It's meant as a failsafe. (In reply to David Walser from comment #4) > The issue was that /etc/X11/xinit.d/xdg-autostart was looking for $DESKTOP > which was sometimes not set (like running startx instead of logging in via a > DM), making xdg-autostart fail to run when it needed to. Ok. In that case my bug report was complete: >> Unfortunately sourcing these files resets DESKTOP to the value of SESSION >> (given as the first argument when sourcing). This is forbidden because it >> breaks the logic of /etc/X11/Xsession. The problem is the line DESKTOP=$1 in combination with "# to be sourced" > OK I've changed it to use the $SESSION variable instead. Please verify that > it fixes your issue and also does not cause IceWM to fail to start > mate-polkit, including when started via startx. Walter, I had a look at 0.1-14 and I think you are moving too fast. I will re-explain: There are two scripts to be adapted, not one: /etc/X11/xinit.d/xdg-autostart /etc/X11/xinit.d/update-menus 1) you have to ask yourself why these files need to be sourced (I know they were already sourced in the past and there might be a hidden reason. Or not...) 2) if it is necessary, or if you don't want to change the sourcing, then indeed it will be better to use $SESSION 3) Your solution is neither 1) or 2). You use $SESSION but no longer source the file. That can't possibly work 4) There is another option where you write a function which returns $1 if present (because of the way Xsession is written. I found a mandriva 2006 which passes $SESSION as an argument), then falls back to $SESSION and maybe ultimately to $DESKTOP > Note that *all* Mageia systems generally have icewm installed unless you > specifically uninstalled it. It's meant as a failsafe. Umm, at least the LiveDVD-KDE4 iso does not have it installed. The other isos I don't know. You are incorrect. I have not changed the fact that the file is sourced, I just removed the DESKTOP=$1, which is what you had complained about, and changed the case statement to look for SESSION instead of DESKTOP, which should already and always be set, so it should work, and fix the problem that the DESKTOP=$1 was originally intended to fix. You are correct that I have not modified update-menus, but we had not modified that in Bug 12015 either, so that's a separate issue. Finally, I was talking about Mageia installations. I was not talking about LiveDVDs. Technically it *should* have IceWM too, but it may not for space reasons. (In reply to David Walser from comment #6) > You are incorrect. I have not changed the fact that the file is sourced, Well, I am not supposed to know how you manage internal versioning. I made the bug report against mga5 (xdg-compliance 0.1-10). If you then patch mga6 and tell me it is fixed I assume you understand the problematic and verified any changes done in between. > I just removed the DESKTOP=$1, which is what you had complained about, and > changed the case statement to look for SESSION instead of DESKTOP, which > should already and always be set, so it should work, and fix the problem > that the DESKTOP=$1 was originally intended to fix. I beg to differ. If you don't source the file then SESSION is just empty. There is a reason why it had been passed as an argument before... From your own bug report mga#12015 and the 0.1-12 src.rpm I see that Atilla ÃNTAÅ had already removed the sourcing + used another variable (DESKTOP_SESSION) which is as far as I can see not present in mga5 (at least not on my system). Maybe you should check with him? Is it compatible with mga5 or will it only work in mga6? In any case: that particular change is also risky. Just reinstating the sourcing mechanism will break it again. Would be better to use "case $DESKTOP_SESSION in" instead. For mga6 that is... > > You are correct that I have not modified update-menus, but we had not > modified that in Bug 12015 either, so that's a separate issue. I'm sorry to remind you that reading my bug report could have helped ;-) But I don't want to start a fight. What surprised me most was that you immediately went patching before discussing the solution and possible consequences. For example: why not putting "case $1 in" and get rid of the DESKTOP/SESSION variable? That way, if anybody sources the file again in the future it won't break anything? Sorry, I had overlooked that the bug report was for Mageia 5. I did see update-menus comes from xdg-compliance, and I removed the DESKTOP=$1 there too and also changed it to use SESSION, but in both cases (also for xdg-autostart), it's not working in Cauldron, because SESSION isn't correctly set. chksession doesn't work at all anymore, since wmsession.d isn't used anymore. I have filed Bug 18681 for this. I had also missed that Atilla removed "# to be sourced" from xdg-autostart, and this was indeed incorrect. I have restored that. Thank you for pointing it out. As for your last point, the file is only meant to be sourced from Xsession, so whether it uses SESSION or $1 is rather irrelevant. CC:
(none) =>
luigiwalser I think this will be fine again in Cauldron once chksession is fixed and you'll be satisfied with that result. As for Mageia 5, barring a good solution that doesn't cause a regression (like, say, for IceWM), I don't know that we'll change it. (In reply to David Walser from comment #8) > I did see update-menus comes from xdg-compliance, and I removed the > DESKTOP=$1 there too and also changed it to use SESSION, but in both cases > (also for xdg-autostart), it's not working in Cauldron, because SESSION > isn't correctly set. chksession doesn't work at all anymore, since > wmsession.d isn't used anymore. I have filed Bug 18681 for this. Thanks. I'll keep an eye on that. > > As for your last point, the file is only meant to be sourced from Xsession, > so whether it uses SESSION or $1 is rather irrelevant. If it's mandatory to source then they are indeed equivalent. I just wasn't sure about this because of Attila's commit. For info: I had a look now at xdg-compliance-0.1-1.mga1 and there it was already sourced. No reason to change that then... (In reply to David Walser from comment #9) > I think this will be fine again in Cauldron once chksession is fixed and > you'll be satisfied with that result. I probably will be :-) > As for Mageia 5, barring a good solution that doesn't cause a regression > (like, say, for IceWM), I don't know that we'll change it. a WONTFIX then? I have no problem with that as long as it is mentioned in the errata. Perhaps add uninstall of xdg-compliance as possible workaround? To me that is satisfactory. Someone capable enough to play with xsession can be reasonably expected to read the doc and use urpme if necessary. Last, I had a look at xdg-compliance-0.1-16. It looks correct. Will test it on mga5 on Tuesday. Hi Walter, another question... might be nothing, but just in case... Do you know Bug 16894? Hahahaha. Note to self: always read your post before hitting send! Sorry for that :-S But yes, I suspected the same for Bug 18681... So I tested 0.1-16 on x86_64 mga5. Works for me Closing as OLD, because Mageia 5 has officially reached its End of Life on December 31st, 2017 https://blog.mageia.org/en/2017/11/07/mageia-5-eol-postponed/ It only continued to get important security updates since then, but non-security bugs have no chance of still getting fixed. Status:
NEW =>
RESOLVED |