Navigation Menu

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

sway: 1.2 -> 1.4, wlroots: 0.8.1 -> 0.10.0 #76787

Merged
merged 8 commits into from Jan 23, 2020
Merged

sway: 1.2 -> 1.4, wlroots: 0.8.1 -> 0.10.0 #76787

merged 8 commits into from Jan 23, 2020

Conversation

primeos
Copy link
Member

@primeos primeos commented Jan 1, 2020

Motivation for this change

Ready for final testing :)
On my setup I've successfully tested sway, swayidle, swaylock, and cage.

nix-review:

8 package were build:
cage sway sway-unwrapped swayidle swaylock swaylock-fancy waybar wlroots

TODOs:

  • Testing (also e.g. waybar and cage)
  • Squash the RC commits before merging
  • The default terminal was replaced with Alacritty (needs to be reflected in the module, we probably have to keep urxvt for backwards compatibility)
  • Check compatibility with wayfire (wayfire: init at 0.3.1 #76759)
    • Doesn't build with wlroots 0.9.1 yet
  • Fix the cage build / request a new upstream release:
    builder for '/nix/store/2az706idl7hd63yfhlvr66vsn6q75yp1-cage-0.1.1.drv' failed with exit code 1; last 10 log lines:
        wlr_data_device_manager_destroy(data_device_mgr);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        wlr_data_device_manager_create
      ../cage.c:385:2: error: implicit declaration of function 'wlr_compositor_destroy'; did you mean 'wlr_cursor_destroy'? [-Werror=implicit-function-declaration]
        wlr_compositor_destroy(compositor);
        ^~~~~~~~~~~~~~~~~~~~~~
        wlr_cursor_destroy
      cc1: all warnings being treated as errors
      [4/9] Compiling C object 'cage@exe/output.c.o'.
      ninja: build stopped: subcommand failed.
    
    WIP: master is fine and I've requested a new release: New release for compatibility with wlroots 0.9.x? cage-kiosk/cage#107
    -> Will use master for now.

Will be done late in a separate PR:

  • Grimshot (should we ship the dependencies by default?)
    • Extra tools in contrib/ aren't installed by default but we could e.g. create an additional package like sway-contrib. (or sway-tools, sway-extras, etc.).
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.
Notify maintainers

cc @

Copy link
Member

@Synthetica9 Synthetica9 left a comment

Choose a reason for hiding this comment

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

Works on my machine 👍

@ghost
Copy link

ghost commented Jan 1, 2020

Hello @primeos : I have tested the PR, and it seems to work properly as of 1.3-rc1. It works flawlessly (and it fixed some issues I had with sway previously).

I can confirm that waybar is working properly.

@ghost
Copy link

ghost commented Jan 2, 2020

@primeos : Should we add grimshot like we've added swaybg in the wrapper ? Maybe add a flag specifically for enabling it ? Maybe we could add a flag also for swaybg, wdyt ?

@Ma27
Copy link
Member

Ma27 commented Jan 2, 2020

Should we add grimshot like we've added swaybg in the wrapper ? Maybe add a flag specifically for enabling it ? Maybe we could add a flag also for swaybg, wdyt ?

I think we should add those by default for convenience. I originally added swaybg to the module after I was fairly confused why swaybg didn't work. In case of a too bloated closure size we could add e.g. a minimal flag (or a sway-minimal).

@ofborg ofborg bot requested review from Synthetica9 and Ma27 January 9, 2020 13:24
@Synthetica9
Copy link
Member

Now crashes with:

2020-01-09 18:01:29 - [EGL] command: eglInitialize, error: 0x3001, message: "DRI2: failed to add configs"
2020-01-09 18:01:29 - [EGL] command: eglInitialize, error: 0x3001, message: "eglInitialize"
2020-01-09 18:01:29 - [render/egl.c:190] Failed to initialize EGL
2020-01-09 18:01:29 - [EGL] command: eglMakeCurrent, error: 0x3008, message: "Invalid display (nil)"
2020-01-09 18:01:29 - [render/wlr_renderer.c:221] Could not initialize EGL
2020-01-09 18:01:29 - [backend/drm/renderer.c:40] Failed to create EGL/WLR renderer
2020-01-09 18:01:29 - [backend/drm/backend.c:203] Failed to initialize renderer
2020-01-09 18:01:29 - [backend/backend.c:198] Failed to open DRM device 9
2020-01-09 18:01:29 - [backend/backend.c:343] Failed to open any DRM device
2020-01-09 18:01:29 - [sway/server.c:47] Unable to create backend

For both 1.3-rc1 and 1.3-rc2. 1.2 works fine.

@primeos primeos mentioned this pull request Jan 10, 2020
10 tasks
@primeos primeos changed the title sway: 1.2 -> 1.3, wlroots: 0.8.1 -> 0.9.0 sway: 1.2 -> 1.3, wlroots: 0.8.1 -> 0.9.1 Jan 10, 2020
@primeos
Copy link
Member Author

primeos commented Jan 10, 2020

@Synthetica9 @elyhaka @Ma27 huge thanks for testing this PR and providing feedback! ❤️

@Synthetica9 strange that both Sway release candidates do now crash :o Was that due to the second wlroots update from 0.9.0 to 0.9.1? On my setup Sway 1.3-rc2 still works fine.

@Synthetica9
Copy link
Member

@Synthetica9 strange that both Sway release candidates do now crash :o Was that due to the second wlroots update from 0.9.0 to 0.9.1? On my setup Sway 1.3-rc2 still works fine.

I think I may have incorrectly tested 1.3-rc1, running 1.2 when I thought I was running 1.3-rc1.

@Synthetica9
Copy link
Member

Filed an upstream issue: swaywm/sway#4898

@ghost
Copy link

ghost commented Jan 11, 2020

Still no issue on my side :

image

Could your issue be GPU-related @Synthetica9 ?

@Synthetica9
Copy link
Member

Could your issue be GPU-related @Synthetica9 ?

I suppose it could be, but I don't suspect it since I have an Intel IGPU laptop. How would I even diagnose that?

@Synthetica9
Copy link
Member

@primeos Could you rebase on master? To get ad8a430

@primeos
Copy link
Member Author

primeos commented Jan 15, 2020

@Synthetica9 good idea, forgot about that, thanks :)

Copy link
Member

@Synthetica9 Synthetica9 left a comment

Choose a reason for hiding this comment

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

... And now it fixed itself again. Current working hypothesis: PEBCAK (somewhere)

@primeos
Copy link
Member Author

primeos commented Jan 18, 2020

(Did another rebase to re-trigger CI as the evaluation was broken.)

And now it fixed itself again. Current working hypothesis: PEBCAK (somewhere)

Cool, glad it works now :) Maybe there where also some other relevant dependency updates in the meantime that fixed it (like the Mesa update in my case).

@ofborg ofborg bot requested a review from Synthetica9 January 18, 2020 22:37
@primeos primeos changed the title sway: 1.2 -> 1.3, wlroots: 0.8.1 -> 0.9.1 sway: 1.2 -> 1.4, wlroots: 0.8.1 -> 0.10.0 Jan 22, 2020
@primeos primeos marked this pull request as ready for review January 22, 2020 20:31
@primeos
Copy link
Member Author

primeos commented Jan 22, 2020

After some testing/reviews this should be finally ready to be merged 🎉.

Regarding cage: 0.1.1 doesn't build with wlroots 0.10.0 and IMO backporting the patches is dangerous (I'm not familiar with the codebase and might miss other important changes, but e.g. FreeBSD seems to do this successfully patch-wlroots-0.9). Therefore I decided to use the master version. An alternative would e.g. be to override wlroots back to the old (compatible) version. Any (strong) opinions regarding this?

Copy link
Member

@Synthetica9 Synthetica9 left a comment

Choose a reason for hiding this comment

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

Works for me *\o/*

@ofborg ofborg bot requested a review from Synthetica9 January 22, 2020 20:57
@ghost
Copy link

ghost commented Jan 22, 2020

Everything's running smoothly on 1.4 :)

@primeos primeos merged commit 5a4b93e into NixOS:master Jan 23, 2020
@primeos
Copy link
Member Author

primeos commented Jan 23, 2020

Great, thanks everyone for the testing and valuable feedback :)

🎉 Merged now 🎉

@NilsIrl
Copy link
Member

NilsIrl commented Jan 26, 2020

I have the same problem as @Synthetica9:

Now crashes with:

2020-01-09 18:01:29 - [EGL] command: eglInitialize, error: 0x3001, message: "DRI2: failed to add configs"
2020-01-09 18:01:29 - [EGL] command: eglInitialize, error: 0x3001, message: "eglInitialize"
2020-01-09 18:01:29 - [render/egl.c:190] Failed to initialize EGL
2020-01-09 18:01:29 - [EGL] command: eglMakeCurrent, error: 0x3008, message: "Invalid display (nil)"
2020-01-09 18:01:29 - [render/wlr_renderer.c:221] Could not initialize EGL
2020-01-09 18:01:29 - [backend/drm/renderer.c:40] Failed to create EGL/WLR renderer
2020-01-09 18:01:29 - [backend/drm/backend.c:203] Failed to initialize renderer
2020-01-09 18:01:29 - [backend/backend.c:198] Failed to open DRM device 9
2020-01-09 18:01:29 - [backend/backend.c:343] Failed to open any DRM device
2020-01-09 18:01:29 - [sway/server.c:47] Unable to create backend

For both 1.3-rc1 and 1.3-rc2. 1.2 works fine.

How can I upgrade mesa? (I try to keep everything related to nixos-unstable using nix-env)

@primeos
Copy link
Member Author

primeos commented Jan 26, 2020

@NilsIrl normally/officially you can only update it system-wide (i.e. update the channel and nixos-rebuild). Using Sway from nixos-unstable on a system based on the stable channel is not really supported in general (it often works anyway, but might result in such problems/incompatibilities).

You could upgrade Mesa independently via setting hardware.opengl.package to an appropriate value (see nixos/modules/hardware/opengl.nix) but that could cause other problems. And there are also other hacks available (e.g. $LD_LIBRARY_PATH).

@NilsIrl
Copy link
Member

NilsIrl commented Jan 26, 2020

I tried setting LD_LIBRARY_PATH but it didn't work.

and from what I can tell, it was fixed on its own for @Synthetica9 so it might have nothing to do with mesa.

@Synthetica9
Copy link
Member

Synthetica9 commented Jan 26, 2020 via email

@NilsIrl
Copy link
Member

NilsIrl commented Jan 26, 2020

Well, running everything under nixos-unstable does fix the problem.

@primeos primeos mentioned this pull request Feb 1, 2020
22 tasks
@evils evils mentioned this pull request May 14, 2020
10 tasks
@primeos
Copy link
Member Author

primeos commented May 16, 2020

FYI: I've opened #87979 regarding the packaging of the tools in contrib/ (mainly Grimshot).

(Thought I'd also mention this here since we discussed it a bit and it was an optional TODO:)

Will be done later in a separate PR:

  • Grimshot (should we ship the dependencies by default?)
    • Extra tools in contrib/ aren't installed by default but we could e.g. create an additional package > like sway-contrib. (or sway-tools, sway-extras, etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants