lightdm-session loads Xresources with "-nocpp" option

Bug #1084885 reported by James E. Flemer
48
This bug affects 9 people
Affects Status Importance Assigned to Milestone
Light Display Manager
Triaged
Medium
Unassigned

Bug Description

LightDM fails to correctly load ~/.Xresources that contains cpp preprocessor directives. In /usr/sbin/lightdm-session, the user's Xresources are loaded via:

    xresourcefile="$HOME/.Xresources"
    if [ -f "$xresourcefile" ]; then
        echo "Loading resource: $xresourcefile"
        xrdb -nocpp -merge "$xresourcefile"
    fi

Why is "-nocpp" there? XDM never used to use "-nocpp", nor does it today (see /etc/X11/Xsession.d/30x11-common_xresources). Having this in the LightDM session init script cripples a rather powerful feature, and causes needless display manager incompatibility.

Perhaps LightDM inherited this bug by copying GDM, which is also broken (see bug #785521).

Revision history for this message
James E. Flemer (jflemer) wrote :

Note, using lightdm-1.2.1-0ubuntu1.1

Revision history for this message
James E. Flemer (jflemer) wrote :

Bug #785521 is for gdm. This bug is for lightdm. It's broken in both packages, therefor I do not think it is a duplicate.

Revision history for this message
Peder Stray (pstray) wrote :

Still not fixed in 1.6.0-0ubuntu3.1 (13.04). This isn't exactly hard to fix, just remove the '-nocpp' and repackage. This seriously breaks my .Xresources

Revision history for this message
Robert Ancell (robert-ancell) wrote :

Correct, copied blindly from gdm.

The change in gdm for this was:

commit 22590328fab9aacf17d4ce32e1c0678c330b26c0
Author: Martin Pitt <email address hidden>
Date: Fri Jul 17 16:13:19 2009 -0400

    Don't invoke cpp with xrdb

    In order to improve startup speed, call xrdb with -nocpp. cpp
    is a pretty heavy beast, and the number of users which have
    preprocessor directives in their ~/.Xresources is probably a
    single digit (as is the number of users who still use
    ~/.Xresources with GNOME in the first place).

Changed in lightdm:
status: New → Triaged
importance: Undecided → Medium
Revision history for this message
Robert Ancell (robert-ancell) wrote :

Martin, can you weigh in if we should or shouldn't use cpp?

Changed in lightdm:
assignee: nobody → Martin Pitt (pitti)
Revision history for this message
Derek Chen-Becker (dchenbecker) wrote :

How bad would it be to simply grep the resource file for cpp directives and decide whether or not to use -cpp/-nocpp based on that info? cpp might be a beast but grep certainly isn't as an initial check.

Revision history for this message
Martin Pitt (pitti) wrote :

-nocpp is being used in gdm/lightdm quite deliberately (not blindly/not a bug). We don't want to penalize the vast majority of users at every boot with wasting the cpp call (which is quite expensive) for the tiny fraction of power users who actually use this.

A check like Derek proposed sounds fine to me, but let's avoid grep please. Posix shell can do substring match just fine:

  test "${string#*$word}" != "$string" && echo "$word is in $string"

Martin Pitt (pitti)
Changed in lightdm:
assignee: Martin Pitt (pitti) → nobody
Revision history for this message
James E. Flemer (jflemer) wrote :

I concur that it is likely a rarely used feature (tho "single digit" seems low). However I think breaking compatibility with a long history of Xdm is an unwise move. To get the best of both worlds (compatibility & feature set, and performance), I would suggest a config option for lightdm. The main app can push that config option into a env var before it calls /usr/sbin/lightdm-session. Then /usr/sbin/lightdm-session can test it. Something like this would work for me:

C code (pseudo code):
   if (config.xrdb_cmd) {
     setenv("LIGHTDM_XRDB_CMD", config.xrdb_cmd, 1);
   }
   ...
   exec("/usr/sbin/lightdm-session" ...);

Shell code (/usr/sbin/lightdm-session):
    xresourcefile="$HOME/.Xresources"
    if [ -f "$xresourcefile" ]; then
        echo "Loading resource: $xresourcefile"
        ${LIGHTDM_XRDB_CMD:-xrdb -nocpp -merge} "$xresourcefile"
    fi

The result is that for all the "simple" users, it's as speedy as ever. For advanced users, its a one time config change to lightdm, and the run-time cost of cpp. Using grep is ugly. Using "test" would be worse (assuming you are proposing sucking the resources contents into a shell var in order to look for something in it).

Revision history for this message
James E. Flemer (jflemer) wrote :

If someone authoratitive (a committer) approves the approach I gave above, I will provide a patch.

Revision history for this message
Martin Pitt (pitti) wrote :

I think a config option is overkill. It doesn't sound hard to check if the ~/.Xresources file contains cpp macros, or is it? (I have zero idea about how they work).

Revision history for this message
James E. Flemer (jflemer) wrote :

An easy way to check if a file contains cpp macros would be to run it through cpp... ;-)

Revision history for this message
jacob kuenzel (jayekub) wrote :

i hit this. luckily i found this post, which directed me here: https://bbs.archlinux.org/viewtopic.php?id=171456

i'm also using the solarized Xresources config, which uses #define directives.

it does appear that -nocpp provides a measurable speedup, so i can understand making it the default. it'd be great for there to be *some* mechanism to change this behavior, though.

Revision history for this message
Pablo Andres Dealbera (sagaro) wrote :

Still present in lightdm 1.18.3. If I can suggest something, maybe an option in the lightdm.conf to give the ability to parse with cpp on, that would be great.

Revision history for this message
James E. Flemer (jflemer) wrote :

Hah. Almost 3 years ago, I suggested a conf option (and offered to write a patch). But my suggestion was shot down by a maintainer, with no alternate solution proposal.

Revision history for this message
felix archambault (archf) wrote :

That is sad. The way it is now we cannot use any of those handy theme: https://github.com/chriskempson/base16-xresources/tree/master/xresources.

I think this would be reasonable feature. Who does want to rewrite these files from scratch. I vote up this issue!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.