NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.21k stars 14.21k forks source link

Kodi on flatpak show the wrong localtime #238386

Closed ipoupaille closed 1 month ago

ipoupaille commented 1 year ago

The time in kodi home page is always shown in UTC. Kodi seem to use TZ env variable to configure the desired timezone (https://github.com/xbmc/xbmc/blob/8f774d10a07b14de5dd9f9984c4e77058bc4e185/xbmc/platform/posix/PosixTimezone.cpp#L190) I tried in a bash shell inside container

flatpak enter $(pidof kodi.bin) bash date => Sun Jun 18 11:31:13 CEST 2023 OK => CEST is 2h more than UTC TZ=CET date => Sun Jun 18 09:31:42 CET 2023 KO => should be Sun Jun 18 11:31:42 CEST 2023 TZ=CEST date => Sun Jun 18 09:31:45 CEST 2023 KO => should be Sun Jun 18 11:31:45 CEST 2023

I tried those commands in another flatpak container (firefox), and the result is the same. I tried it in kodi flatpak with another OS (ubuntu 22.04), and all the commands are working correctly (date in desired timezone) I tried it directly on nixos, and the date is correctly shown with TZ=CET, but not with TZ=CEST. I tried it directly on ubuntu 22.04, and all the commands are working correctly

My nixos channel is https://nixos.org/channels/nixos-23.05

ipoupaille commented 1 year ago

Found something, If I set the env variable TZDIR=/usr/share/zoneinfo, this is working for TZ=CET in flatpak

ipoupaille commented 1 year ago

Anyone?

ipoupaille commented 11 months ago

Perhaps I forgot that: @jtojnar

claes commented 2 months ago

This is troubling me too. When I enter bash using flatpak enter $(pidof kodi.bin) bash I see that TZDIR is set to /etc/zoneinfo and this directory does not exist within this shell session (within the sandbox)

claes commented 2 months ago

In can confirm that starting Kodi like

flatpak run --env=TZDIR=/usr/share/zoneinfo tv.kodi.Kodi

presents the right time.

claes commented 2 months ago

/usr/share/zoneinfo is the directory where the Flatpak environment org.freedesktop.Platform provides the timezone files.

$ flatpak run --command=bash --devel org.freedesktop.Platform
[📦 org.freedesktop.Platform ~]$ ls /usr/share/zoneinfo
Africa     Asia       CET   (etc..)

and if I start Kodi with flatpak run --unset-env=TZDIR tv.kodi.Kodi

it also displays the right time. So I think the issue is that the TZDIR env variable is inherited from Nix environment

claes commented 2 months ago

Related https://github.com/flatpak/flatpak/issues/4715 as reported by Guix dev

I think it would work if nixpkgs adds TZDIR to the env variables to unset in https://github.com/NixOS/nixpkgs/blob/master/pkgs/by-name/fl/flatpak/unset-env-vars.patch

ipoupaille commented 2 months ago

I entered this command once : flatpak override --env=TZDIR=/usr/share/zoneinfo And it is working every time. No need to do something with the run command

But I agree that it should be done elsewhere (unset-env-vars seem to be great)

claes commented 2 months ago

@getchoo any input on the above?

claes commented 2 months ago

Looks like this was recently fixed in Flatpak

https://github.com/flatpak/flatpak/pull/5850

claes commented 2 months ago

In 1.15.9 https://github.com/flatpak/flatpak/releases/tag/1.15.9

getchoo commented 1 month ago

@getchoo any input on the above?

Sorry for the wait, I could've sworn I responded to this 😆

I've opened https://github.com/NixOS/nixpkgs/pull/343590 with the mentioned patch. Thanks for the report!

claes commented 1 month ago

Thanks for looking into this!

When running Kodi using the patched nixpkgs Flatpak, I don't get the expected effect. I am not sure why. I see that the original Flatpak fix attempts to use TZDIR to do some ro-bind with bubblewrap but I don't see that argument when I look at the output of ps so perhaps the issue is in the original Flatpak fix.

I find this part of the Flatpak fix strange

https://github.com/swick/flatpak/blob/7c8a81f08908019bbf69358de199748a9bcb29e3/common/flatpak-run.c#L1614

"--ro-bind", tzdir, "/usr/share/zoneinfo",

If I understand it right, it attempts to mount the TZDIR directory below /usr/share/zoneinfo But with TZDIR set, applications will try to look under TZDIR under the value of TZDIR not under /usr/share/zoneinfo

So unless I misunderstand something I think the original Flatpak fix may not work as intended (which would not be a nixpkgs problem but affect nixpkgs)

Aleksanaa commented 1 month ago

"--ro-bind", tzdir, "/usr/share/zoneinfo",

If I understand it right, it attempts to mount the TZDIR directory below /usr/share/zoneinfo

My understanding is it's linking the path of $TZDIR (instead of /usr/share/zoneinfo on host) to /usr/share/zoneinfo inside bubblewrap, not passing $TZDIR environment variable into it (I don't see the relevant code. However this may have been implemented elsewhere)

claes commented 1 month ago

Yes I think it does this too. But I think it does not address the problem that occurs on NixOS. If I start Kodi using the patched Flatpak like this

./result/bin/flatpak run tv.kodi.Kodi and then

$ result/bin/flatpak enter $(pidof /app/lib/kodi/kodi.bin) bash
bash-5.2$ echo $TZDIR
/etc/zoneinfo
bash-5.2$ ls $TZDIR
ls: cannot access '/etc/zoneinfo': No such file or directory

We can see that TZDIR, as passed into the Flatpak sandbox by NixOS is not present in the sandbox

When asking date for current time in my current timezone first without timzone and then with my timezone, I can see it differs. But when passing a path inside the sandbox that exists there, it works

bash-5.2$ date
Sun Sep 22 11:24:34 CEST 2024
bash-5.2$ TZ=CET date
Sun Sep 22 09:24:29 CET 2024
bash-5.2$ TZDIR=/usr/share/zoneinfo TZ=CET date
Sun Sep 22 11:24:37 CEST 2024

I think this happens because date comment attempts to access timezone files in TZDIR=/etc/zoneinfo which is not mounted into the sandbox. I would have assumed the original Flatpak patch attempted to do that, but it does something else. So I think the original patch is not addressing the issue that occurs in NixOS. I don't know what it does actually

claes commented 1 month ago

I think there are two possible paths forward for NixOS: Either don't pass TZDIR into the sandbox and let the default be used instead, or by mounting the contents of NixOS TZDIR into TZDIR inside the sandbox.

The first option would use Flatpak's own timezone files for timezone lookup. The second would use what NixOS uses. Ideally Flatpak would have solved it the second way in their fix but it seems it does not really do that

Aleksanaa commented 1 month ago

Try:

  1. Is /etc/profile available inside flatpak?
  2. Try setting environment.sessionVariables.FOO_BAR = "bar", is that also available inside flatpak?
claes commented 1 month ago
  1. No it is not

  2. Yes it is

./result/bin/flatpak enter $(pidof /app/lib/kodi/kodi.bin) bash
bash-5.2$ set |grep -i foo
FOO_BAR=bar
bash-5.2$ ls /etc/profile
ls: cannot access '/etc/profile': No such file or directory
bash-5.2$ set |grep -i foo
FOO_BAR=bar
Aleksanaa commented 1 month ago

Good, now we'll have another patch dedicated to Nixpkgs.

https://github.com/NixOS/nixpkgs/blob/3f423168c9cf47ac94483ea0491ca6cc28f73c9a/nixos/modules/config/locale.nix#L73

jtojnar commented 1 month ago

If I understand it right, it attempts to mount the TZDIR directory below /usr/share/zoneinfo But with TZDIR set, applications will try to look under TZDIR under the value of TZDIR not under /usr/share/zoneinfo

I think that is right. Flatpak should addTZDIR to blacklist (i.e. this) upstream, now that TZDIR from the host is properly mounted to /usr/share/zoneinfo in the guest.

claes commented 1 month ago

https://github.com/NixOS/nixpkgs/pull/343704 is approved and awaiting merge. @Aleksanaa would you mind having a look?