env: set XDG_DATA_DIRS for wayland et.al.

[sergiusens]

Currently XDG_DATA_DIRS is only setup for an X session, there
is currently no specific location to set this up for anything
using the wayland protocol. The profile.d mechanism seems like
the next best choice.

LP: #1681547

Signed-off-by: Sergio Schvezov sergio.schvezov@ubuntu.com

[mvo5]

Thanks, nice catch!

[zyga]

+1 on the effort but I think this needs to be managed better across distributions and other files that serve the same purpose (though get sourced in different ways)

CC @morphis

[zyga]

Is this valid sh (not bash) syntax?

[ogra1]

yes, thats appropriate to split variable content

[morphis]

This very much looks like a copy of https://github.com/snapcore/snapd/blob/master/etc/X11/Xsession.d/65snappy

Can't we link both somehow?

[chipaca]

What's the status of this PR?

[ogra1]

since this currently blocks snap usage in wayland altogether and since wayland wont ever parse /etx/X11/Xsession.d (i was told) ... i'm approving this as interim solution

[jdstrand]

+1 on the contents of the file, but -1 on the name of the file.

'snapd.sh' is actually referenced in ./packaging/opensuse-42.2/snapd.spec and ./packaging/arch/PKGBUILD.

I suggest renaming the file 'snapd-xdg-data-dirs.sh'.

Also, can you update ./packaging/opensuse-42.2/snapd.spec and ./packaging/arch/PKGBUILD to do for this file what they are doing for apps-bin-path.sh?

[chipaca]

Unless anybody objects I'm going to take over this PR, and unify all this into a single file. Ubuntu and Debian would ship it as /etc/profile.d/apps-bin-path.sh (because that's what we ship today, it's picked up by Xsession, and it's a conffile, and renaming conf files is AFAIK LOLNO territory), and it'd replace what's currently in snapd.spec and arch/snapd.sh (although tbh it might look alarmingly light the latter)

[ogra1]

/etc/profile.d is not picked up by Xsession, it only applies to shells under X11 (but due to a hack is read for wayland sessions)...
for plain X11 the existing /etc/X11/Xsession.d/65snappy still needs to persist

[chipaca]

are you sure profile.d isn't picked up by X? I moved 65snappy to /etc/profile.d/snapd.sh and started unity, xfce and jwm (heh) and in all cases ps eww of the toplevel process showed XDG_DATA_DIRS having snapd in it

[ogra1]

"ps eww of the toplevel process" is surely a shell in this case ... apps are usually not executed under a shell but directly, you would have to dump the env out of a C app that is launched from a .desktop file to find out ...

[chipaca]

I mean I do this:

~$ ps fx
  PID TTY      STAT   TIME COMMAND
16758 ?        Ss     0:00 /sbin/upstart --user
16845 ?        S      0:00  \_ upstart-udev-bridge --daemon --user
16852 ?        Ss     0:04  \_ dbus-daemon --fork --session --address=unix:abstract=/tmp/dbus-qFtZh3OQOM
16885 ?        S      0:00  \_ upstart-dbus-bridge --daemon --system --user --bus-name system
...

so then

~$ ps eww 16758 | egrep -o 'XDG_DATA_DIRS=[^ ]*'
XDG_DATA_DIRS=/usr/share/ubuntu:/usr/share/gnome:/usr/local/share/:/usr/share/:/var/lib/snapd/desktop

[pedronis]

@chipaca is working on adjusting this

[mvo5]

Thanks for your work on this, this looks much nicer than before.

[mvo5]

Hm, could we have a single make -C data clean here?

[mvo5]

Could we have a single "$(MAKE) -C data all" here?

[chipaca]

14.04 wasn't making dbus. Is that a bug?

[mvo5]

Probably :/

[mvo5]

Same question as for the 14.04 branch

[mvo5]

Fwiw, both lightdm and gdm3 load /etc/profile which in turn will load /etc/profile.d (in gdm3-3.24.1/data/Xsession.in and lightdm-1.22.0/debian/lightdm-session).

[jdstrand]

I like the direction of this PR much more. Thanks for addressing my concerns (and more!) :)

[mvo5]

❤️

[mvo5]

❤️ ❤️

[niemeyer]

@chipaca Thanks for working on this. The current error looks related:

+ echo 'Ensure sudo works without a password inside classic'
Ensure sudo works without a password inside classic
++ tr -d '\r'
++ classic sudo id -u
+ '[' '/etc/profile: line 23: .: /etc/profile.d/apps-bin-path.sh: is a directory
0' '!=' 0 ']'
+ echo 'sudo inside classic did not work as expected'
sudo inside classic did not work as expected

[niemeyer]

Once tests pass...

[chipaca]

@niemeyer yes, looking into why we're getting a directory there. Probably some silly error on my end.

[codecov-io]

Codecov Report

Merging #3398 into master will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #3398      +/-   ##
=========================================
+ Coverage   75.63%   75.7%   +0.06%     
=========================================
  Files         407     407              
  Lines       35028   35268     +240     
=========================================
+ Hits        26494   26698     +204     
- Misses       6654    6687      +33     
- Partials     1880    1883       +3

Impacted Files	Coverage Δ
interfaces/builtin/i2c.go	65.95% <0%> (-1.39%)	⬇️
interfaces/builtin/iio.go	69.64% <0%> (-0.54%)	⬇️
interfaces/builtin/framebuffer.go	100% <0%> (ø)	⬆️
interfaces/builtin/account_control.go	100% <0%> (ø)	⬆️
interfaces/builtin/udisks2.go	100% <0%> (ø)	⬆️
interfaces/builtin/tpm.go	100% <0%> (ø)	⬆️
interfaces/builtin/kernel_module_control.go	100% <0%> (ø)	⬆️
interfaces/builtin/bluetooth_control.go	100% <0%> (ø)	⬆️
interfaces/builtin/broadcom_asic_control.go	100% <0%> (ø)	⬆️
interfaces/builtin/optical_drive.go	100% <0%> (ø)	⬆️
... and 20 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1182f06...4217778. Read the comment docs.