[Bug 1955850] Re: /etc/X11/Xresources no longer loaded, error in patch

[Andreas Hasenack]

I posted a comment in the PR above, and I reproduce it here:

"""
I have a question about replacing the `expr "$i" : '^[[:alnum:]_-]\+$'` check with `find -type f`. It seems we would start to include files with a dot in them with this change:

```
$ l /etc/X11/Xresources/*
-rw-r--r-- 1 root root 319 Oct  9  2020 /etc/X11/Xresources/x11-common

$ expr "x11-common" : '^[[:alnum:]_-]\+$' > /dev/null; echo $?
0

$ expr "x11-common.dpkg-confold" : '^[[:alnum:]_-]\+$' > /dev/null; echo $?
1
```

Since `$sysresources` points `/etc/X11/Xresources`, there could conceivably be backup config files in there, which the original expression would be ignoring, but not the new `find`. Has this been considered, or am I wrong in my analysis?
"""

-- 
You received this bug notification because you are a member of Ubuntu
Sponsors, which is subscribed to the bug report.
https://bugs.launchpad.net/bugs/1955850

Title:
  /etc/X11/Xresources no longer loaded, error in patch

Status in gdm3 package in Ubuntu:
  In Progress

Bug description:
  After upgrading from 21.04 to 21.10, my /etc/X11/Xresources/ directory
  no longer loads on login.

  Looking at the sources, I think this can be traced to:
   - Switching from a custom Xsession file to the upstream Xsession file in 3.38.0-1ubuntu1, combined with
   - An oversight in `debian/patches/ubuntu/xresources_is_a_dir.patch`

  Looking at [that patch](https://salsa.debian.org/gnome-
  team/gdm/-/blob/ubuntu/master/debian/patches/ubuntu/xresources_is_a_dir.patch),
  it has:

  ```diff
  --- a/data/Xsession.in
  +++ b/data/Xsession.in
  @@ -122,6 +122,13 @@ fi
   
   if [ -f "$sysresources" ]; then
       xrdb -nocpp -merge "$sysresources"
  +elif [ -d "$sysresources" ]; then
  +    # directory of Xresources files on Debian/Ubuntu
  +    for i in `ls "$sysresources"` ; do
  +        if [ -r "$sysresources/$i"  -a -f "$sysresources/$i" ] && expr "$i" : '^[[:alnum:]_-]\+$' > /dev/null; then
  +            xrdb -nocpp -merge "$sysresources/$i"
  +        fi
  +    done
   fi
   
   if [ -f "$userresources" ]; then
  ```

  However, looking a few lines further up in [the original Xsession.in
  file](https://salsa.debian.org/gnome-
  team/gdm/-/blob/ubuntu/master/data/Xsession.in#L97), I see:

  ```sh
  if [ -f /etc/X11/Xresources ]; then
      sysresources=/etc/X11/Xresources
  else
      sysresources=/usr/etc/X11/Xresources
  fi
  ```

  In other words, if `/etc/X11/Xresources` is a directory, the
  `sysresources` variable is never set correctly, so the patch should
  also modify this part of the script.

  I have observed this broken behavior on version 41~rc-0ubuntu2, but
  looking at the ubuntu/master branch (as linked above), it still seems
  broken there.

  I'm currently testing this patch:

  ```diff
  --- Xsession.orig       2021-12-27 22:05:09.038863375 +0100
  +++ Xsession    2021-12-27 22:05:33.143057433 +0100
  @@ -111,7 +111,7 @@
   usermodmap="$HOME/.Xmodmap"
   userxkbmap="$HOME/.Xkbmap"
   
  -if [ -f /etc/X11/Xresources ]; then
  +if [ -e /etc/X11/Xresources ]; then
       sysresources=/etc/X11/Xresources
   else
       sysresources=/usr/etc/X11/Xresources
  ```

  I don't know if it works yet, since I need to log out for that, and
  close my browser, so need to submit this report first ;-)

  ```
  $ lsb_release -rd
  Description:    Ubuntu 21.10
  Release:        21.10
  $ apt-cache policy gdm3
  gdm3:
    Installed: 41~rc-0ubuntu2
    Candidate: 41~rc-0ubuntu2
    Version table:
   *** 41~rc-0ubuntu2 500
          500 http://nl.archive.ubuntu.com/ubuntu impish/main amd64 Packages
          100 /var/lib/dpkg/status
  ```

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/gdm3/+bug/1955850/+subscriptions