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.

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

nixosTests.gnome3-flashback: init #71212

Closed
wants to merge 3 commits into from

Conversation

worldofpeace
Copy link
Contributor

Motivation for this change

Fixes #70561.

Test script is identical to the ancient one we have for gnome3-xorg, but at least by having this we'll catch things like #71210, and just testing the session is convenient on GitHub.

Has #71210 because you can't actually test the tests without it.
(please merge that first).

Things done

Haven't tested the tests yet 😄, as I have to run all three of them.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@worldofpeace
Copy link
Contributor Author

worldofpeace commented Oct 16, 2019

Perhaps I should move the session tests into a subdir gnome3

flashback.nix
xorg.nix
default.nix (wayland default)

I might also want to hold off merging right away, the test script could be more flashback specific and check if those components are ok.

@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) This PR changes an existing module in `nixos/` labels Oct 16, 2019
@worldofpeace
Copy link
Contributor Author

@GrahamcOfBorg eval


services.xserver.enable = true;

services.xserver.displayManager.gdm.enable = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a comment why this is done? I do not think git blame will carry to the original file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it seems in the gnome3-xorg test, and in the past gnome3 module enabled gdm and then it got removed and this got added. And I've copied it here.

Actual issue with why we don't test with gdm is #66443.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, link there should be enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GDM doesn't have any way to specify a default session, and I'm guessing it will happily launch wayland sessions even though GDM itself runs X11. Think I switched the test to LightDM when adding wayland support since I didn't manage to get it launch the correct session.

We'd probably have to do something with AccountsService to get it working.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, that's weird, GDM X11 does indeed launch an X11 session, never mind me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I wasn't sure why I was seeing error: unable to open display ":0.0" when using xwininfo. Only thing I thought of was it using wayland? But it was explicitly disabled.

Copy link
Contributor Author

@worldofpeace worldofpeace Nov 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hedning You're actually correct, setting xserver default will have no effects on gdm.
So we do need to set it with accountsservice over dbus.

@worldofpeace
Copy link
Contributor Author

Ok, so the feature to use different wm with gnome-flashback doesn't work as expected.

So for example, with i3, it doesn't work without i3 installed because it needs its desktop file to launch required components. And even if I do this, none of the flashback components are visible (or maybe I just don't understand i3). And with xmonad there's no desktop file so it shows the "fail whale" requesting you to logout. (and gnome-flashback components are tiled)

I don't really get why things have to be done in this way, can't you just do gnome-flashback+wm with x11?

@jtojnar Has this ever worked?

@jtojnar
Copy link
Member

jtojnar commented Oct 19, 2019

I am pretty sure it worked.

We are creating the desktop file here:

https://github.com/NixOS/nixpkgs/pull/53695/files#diff-eccb87703abad3f98a3c3e5c15584083R110-R125

@worldofpeace
Copy link
Contributor Author

Weird... I'll look closer, perhaps it has something to do with systemd sessions.

@jtojnar
Copy link
Member

jtojnar commented Oct 19, 2019

I don't really get why things have to be done in this way, can't you just do gnome-flashback+wm with x11?

The idea of #39871 was we would get rid of the old xserver module and just use DMs with session files.

worldofpeace added a commit to worldofpeace/nixpkgs that referenced this pull request Oct 19, 2019
This reverts commit 60aedad.

Using tests from NixOS#71212 I am now unable to reproduce there being issues
with starting the default metacity flashback session without this.
@worldofpeace
Copy link
Contributor Author

Ok, so testing with bspwm appears to give some more apparent failures.

[  121.326242] gnome-session[851]: gnome-session-binary[851]: WARNING: Application 'bspwm.desktop' failed to register before timeout
[  121.333065] gnome-session-binary[851]: Unrecoverable failure in required component bspwm.desktop
[  121.338569] gnome-session-binary[851]: WARNING: Application 'bspwm.desktop' failed to register before timeout

^ that happens and we get the fail whale, though I can clearly see bspwm running fine.

@worldofpeace
Copy link
Contributor Author

Going to quote what I told @jtojnar in a pm on IRC

btw, just tested gnome-flashback with bspwm and there's no issues with gnome-session 3.32. in 3.34 it's broken though
so maybe autostarts and non-systemd sessions probably have some sort of regression

So I'm thinking because we see the message

WARNING: Application 'bspwm.desktop' failed to register before timeout

it considers bspwm.desktop an Application, even though we set it to start at the windowmanager phase. So somehow in 3.34.x X-GNOME-Autostart-Phase isn't working?

peti pushed a commit that referenced this pull request Oct 21, 2019
This reverts commit 60aedad.

Using tests from #71212 I am now unable to reproduce there being issues
with starting the default metacity flashback session without this.
These are terminal tests identical to the gnome3-xorg
test for various gnome flashback configurations.
@worldofpeace
Copy link
Contributor Author

@jtojnar I disabled the other tests and just made the metacity one available. We should get to reporting the issues with those sessions to gnome-session eventually. (hopefully before 20.03).

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild and removed 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 6, 2019
@worldofpeace
Copy link
Contributor Author

@GrahamcOfBorg test gnome3-flashback.metacity

@@ -0,0 +1,98 @@
# GNOME Flashback Terminal tests for various custom sessions.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Terminal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, they pretty much test if a terminal shows up.
Btw, I've pretty much rewritten this entirely locally. Though there's issues with the python testing driver and other ports preventing me from re-PRing it.

@stale
Copy link

stale bot commented Jun 1, 2020

Thank you for your contributions.
This has been automatically marked as stale because it has had no activity for 180 days.
If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.
Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the
    related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse. 3. Ask on the #nixos channel on
    irc.freenode.net.

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 1, 2020
@worldofpeace
Copy link
Contributor Author

yep, it's stale. I have a branch with it rewritten, just haven't gotten to PR'ing.

@worldofpeace worldofpeace deleted the gnome-flashback-test branch June 18, 2020 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: nixos 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add NixOS test for Gnome Flashback
3 participants