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

Jump to bottom

nixos/xfce4-14: init #66859

Merged
merged 3 commits into from Aug 28, 2019
Merged

nixos/xfce4-14: init #66859

merged 3 commits into from Aug 28, 2019

Conversation

worldofpeace
Copy link
Contributor

Motivation for this change

Based off and completes #37594.

Ideally we would have one xfce module and package set, but things aren't as simple as that.
We have 7af5c81 to prevent both of the modules from being used simultaneously, as I'm assuming this could be dangerous.

The following is a quick summary of the changes between #37594

  • enable required DBus services
  • LibInput is enabled
  • add a polkit authentication agent
  • only use xfce4-pulseaudio-plugin for pulseaudio support in panel (the other one was extra)
  • use proper package set for network manager packages
  • use services.xserver.displayManager.sessionCommands over extraSessionCommands
  • configuration for Systemd user services

Lastly we dropped xfce4-mixer because it's abandoned.

Further work/Questions

I haven't resynced the thunar plugins patch, not sure if there's compatible plugins at this version?
And should networkmanager be default enabled?

Things done

Checked in a VM that with this configuration, we could get past the login and at least show the desktop. No glaring issues are apparent to me.

  • 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.
Notify maintainers

cc @

@worldofpeace
Copy link
Contributor Author

Dropped the assertion since there's no way to rule that out completely, and synced this with module changes I've made on master.

jtojnar
jtojnar reviewed Aug 20, 2019
View reviewed changes
Copy link
Contributor

@jtojnar jtojnar left a comment

Choose a reason for hiding this comment

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

hmm, I forgot to submit this.

nixos/modules/services/x11/desktop-managers/xfce4-14.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/xfce4-14.nix Outdated Show resolved Hide resolved
nixos/modules/services/x11/desktop-managers/xfce4-14.nix
environment.pathsToLink = [
"/share/xfce4"
"/lib/xfce4"
"/share/gtksourceview-3.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably move this into a separate module.

nixos/modules/services/x11/desktop-managers/xfce4-14.nix Outdated Show resolved Hide resolved
@worldofpeace
Copy link
Contributor Author

Safe coexisting of different xfce (up to their simultaneous run, say one on the attached display and another via xrdp) could be done by adding an option to change ~/.config/xfce4.
Not sure if we have to bring it to nixpkgs

Not sure if it would be worth it, I plan if possible to deprecate the previous version next release.

@worldofpeace worldofpeace added this to the 19.09 milestone Aug 21, 2019
@worldofpeace
nixos/xfce4-14: init
ae1cb0b
@worldofpeace
xfce4-14.xfce4-mixer: drop
de89ad1 
obsoleted by xfce4-pulseaudio-plugin / dead package
@worldofpeace
nixosTests.xfce4-14: init
fd7d31b 
This is pretty much identical to the xfce test we currently have.
@worldofpeace
Copy link
Contributor Author

Added a copy cat test of the one we already have so this can be in the tested jobset.

romildo
romildo approved these changes Aug 23, 2019
View reviewed changes
Copy link
Contributor

@romildo romildo left a comment

Choose a reason for hiding this comment

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

Tested on a VM. Looks good to me.

@worldofpeace
Copy link
Contributor Author

The only last case I think we should be concerned about is the closure size vs the older version of xfce.
I'm guessing users of xfce would like things to be lightweight and I'd like to be sure it's still acceptable by that standard.

@worldofpeace worldofpeace merged commit 27a4afe into NixOS:master Aug 28, 2019
@worldofpeace worldofpeace deleted the xfce4-14-module branch August 28, 2019 02:37
@worldofpeace worldofpeace mentioned this pull request Aug 28, 2019
Merged
10 tasks
@worldofpeace
Copy link
Contributor Author

Are we also sure that with nixpkgs-update it won't send pr's updating things to release candidates/unstable releases? (i.e properly version ignored).

@jtojnar
Copy link
Contributor

jtojnar commented Sep 1, 2019

For GNOME, I regularly check my packages and flag anything that is unstable. Usually, I catch them pretty soon after they appear in Fedora Rawhide, preventing nixpkgs-update from picking them up. After two release cycles, almost every package related to GNOME platform has an ignore pattern on Repology, so only few infrequently released packages remain to be flagged in the future. I guess something similar will need to be done for Xfce.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar left review comments

@jerome-diver jerome-diver jerome-diver left review comments

@romildo romildo romildo approved these changes

@lukateras lukateras Awaiting requested review from lukateras

Assignees
No one assigned
Labels
6.topic: nixos 6.topic: xfce The Xfce Desktop Environment 8.has: clean-up 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
19.09
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@worldofpeace @jtojnar @romildo @jerome-diver