Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

nixos: make GIO_EXTRA_MODULES a session variable, take two #151105

Merged
merged 3 commits into from
Feb 2, 2022
Merged

nixos: make GIO_EXTRA_MODULES a session variable, take two #151105

merged 3 commits into from
Feb 2, 2022

Conversation

ncfavier
Copy link
Member

@ncfavier ncfavier commented Dec 17, 2021 •
edited
Loading

Another attempt at #126832, which resulted in GDM crashing because of https://gitlab.gnome.org/GNOME/gdm/-/issues/756. As a workaround until that bug is fixed, wrap gnome-session-binary with an empty GIO_EXTRA_MODULES.

GDM and GNOME seem to work fine, although some more testing would be welcome.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.05 Release Notes (or backporting 21.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) labels Dec 17, 2021
@ofborg ofborg bot requested review from dasj19 and amaxine December 17, 2021 17:21
@ncfavier ncfavier mentioned this pull request Dec 20, 2021
Closed
@ncfavier
Copy link
Member Author

ncfavier commented Jan 14, 2022 •
edited
Loading

@jtojnar what are your thoughts on this? I know it's a hack but I don't think it's worth waiting for upstream to fix the issue

I'm also not sure what more we can do to test it

@jtojnar
Copy link
Member

jtojnar commented Jan 14, 2022

Sounds okay to me but we will need to do more thorough testing to make sure it does not break anything else.

@ncfavier
Copy link
Member Author

I've tested basic functionality (log in, open a file browser) for the major display managers (lightdm, gdm, sddm) and desktop environments (gnome, plasma5, xfce, enlightenment, cinnamon, lxqt, mate) in a VM with programs.dconf, services.gvfs and services.gnome.glib-networking all enabled. Also since this is mostly a gnome thing I spent a bit more time fiddling around in gdm and gnome but didn't find any obvious issues.

Would this be enough or should we do some kind of call for testers?

@ncfavier ncfavier marked this pull request as draft January 15, 2022 15:04
@ncfavier
Copy link
Member Author

ncfavier commented Jan 15, 2022 •
edited
Loading

Less hacky fix following upstream discussion: allow gnome-session to find GDM's gnome-login.session.

@ofborg test gnome gnome-xorg

(I've confirmed that this fixes GDM, and it doesn't affect anything else)

@ncfavier ncfavier marked this pull request as ready for review January 15, 2022 20:27
@jtojnar
Copy link
Member

jtojnar commented Jan 15, 2022

Sounds good to me.

@ncfavier
Copy link
Member Author

Added an explanation in a comment.

@ncfavier
Copy link
Member Author

ncfavier commented Jan 16, 2022 •
edited
Loading

@ofborg test gnome gnome-xorg
(using the new test from #150980 now)

@jtojnar
Copy link
Member

jtojnar commented Jan 16, 2022

Looks like it is failing now: https://logs.nix.ci/?key=nixos/nixpkgs.151105&attempt_id=ac824d64-88cc-401f-9c60-8027adc68f6c

@ncfavier
Copy link
Member Author

Weird. I was only able to reproduce it once, now it's always succeeding

@ofborg test gnome

@ncfavier
Copy link
Member Author

Still failing, still succeeding locally on the same commit. I hate computers.

@ncfavier
Copy link
Member Author

So the error seems to be this:

machine # [   85.194565] .gnome-shell-wr[974]: JS ERROR: TypeError: loginItem is null
machine # _startTimedLogin/tasks<@resource:///org/gnome/shell/gdm/loginDialog.js:1072:17
machine # run@resource:///org/gnome/shell/gdm/batch.js:62:33
machine # runTask@resource:///org/gnome/shell/gdm/batch.js:134:51
machine # process@resource:///org/gnome/shell/gdm/batch.js:193:25
machine # nextTask@resource:///org/gnome/shell/gdm/batch.js:151:14
machine # process@resource:///org/gnome/shell/gdm/batch.js:204:18
machine # _start@resource:///org/gnome/shell/gdm/batch.js:159:14
machine # run@resource:///org/gnome/shell/gdm/batch.js:163:14
machine # _startTimedLogin@resource:///org/gnome/shell/gdm/loginDialog.js:1124:38
machine # _onTimedLoginRequested@resource:///org/gnome/shell/gdm/loginDialog.js:1131:14

Google shows no hits.

I'm going to try to run the test a couple more times, sorry for the noise
@ofborg test gnome

@ncfavier
Copy link
Member Author

Succeeds now. Looking at the source code this seems to be a timing issue: loginItem is null, probably because the _timedLoginUserListHold hold isn't being waited on, because the first task doesn't actually return the hold (this was changed in https://gitlab.gnome.org/GNOME/gnome-shell/-/commit/11965324938bc99e1507682b6f9db1eeaef1eeeb#16ed50f668d0ddd1464a281f6a1ddcaf482d5bd8_1052_1055 8 months ago). I will PR upstream with a fix, in the meantime maybe we should increase the autologin delay to 2 or 3 seconds.

I'm confident that the issue isn't related to this PR, though it is related to #150980 since this is specific to timed logins.

@ncfavier
Copy link
Member Author

Opened https://gitlab.gnome.org/GNOME/gnome-shell/-/merge_requests/2116

ncfavier added a commit to ncfavier/nixpkgs that referenced this pull request Jan 18, 2022
@ncfavier
nixosTests.gnome: increase autologin delay
720a88b 
Increase the autologin delay to 3 seconds in the hope to avoid some
failures, see NixOS#151105 (comment)
@ncfavier ncfavier mentioned this pull request Jan 18, 2022
Closed
@ncfavier
nixos/gdm: add gdm to XDG_DATA_DIRS
d25ffc3 
This is necessary so that gnome-session can find GDM's gnome-login.session,
see https://gitlab.gnome.org/GNOME/gdm/-/issues/756
@ncfavier
nixos: make GIO_EXTRA_MODULES a session variable
23cfef7 
Allow applications started by the systemd user session manager to find
their GIO_EXTRA_MODULES.
@ncfavier
Copy link
Member Author

@ofborg test gnome gnome-xorg

@ncfavier
Copy link
Member Author

@ofborg test gnome

1 similar comment
@ncfavier
Copy link
Member Author

@ofborg test gnome

@ncfavier
Copy link
Member Author

Let's see how the logs compare
@ofborg test gnome

@ncfavier
Copy link
Member Author

Yeah this is just duplicated noise

machine # [    5.492191] systemd[1]: Starting Journal Service...
machine #          Starting Journal Service...

@ncfavier
Copy link
Member Author

Should be all good now
@ofborg test gnome
@ofborg test gnome-xorg

@ncfavier ncfavier requested a review from jtojnar January 23, 2022 13:10
@ncfavier
Copy link
Member Author

Now that https://gitlab.gnome.org/GNOME/gdm/-/merge_requests/168 is merged, do you want to wait for the next GDM release so we can skip d25ffc3 ?

@jtojnar
Copy link
Member

jtojnar commented Jan 27, 2022

Not sure if it makes sense to wait https://wiki.gnome.org/FortyTwo

@ncfavier
Copy link
Member Author

Let's not wait then.

@ncfavier
Copy link
Member Author

ncfavier commented Feb 2, 2022

What are we waiting for?

@jtojnar jtojnar merged commit 16658b7 into NixOS:master Feb 2, 2022
@ncfavier ncfavier deleted the gio-extra-modules branch February 2, 2022 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hedning hedning Awaiting requested review from hedning

@dasj19 dasj19 Awaiting requested review from dasj19

@amaxine amaxine Awaiting requested review from amaxine

@jtojnar jtojnar Awaiting requested review from jtojnar

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ncfavier @jtojnar