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

Fix services.xserver.extraLayouts under GNOME 3 #76591

Merged
merged 3 commits into from Dec 28, 2019
Merged

Fix services.xserver.extraLayouts under GNOME 3 #76591

merged 3 commits into from Dec 28, 2019

Conversation

bbjubjub2494
Copy link
Member

@bbjubjub2494 bbjubjub2494 commented Dec 27, 2019 •
edited

Motivation for this change

Fix #76590

Things done
  • Alter the extra-layouts module to set an existing conventional environment variable.
  • Forward-port a patch to libgnomedesktop to honor it. It will be in upstream 3.36 but I understand that Markhor will ship with 3.34.

Targeting staging because it cascades through lots of GNOME derivations unfortunately.

  • Function-tested in 19.09 and unstable
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS inapplicable
    • other Linux distributions inapplicable
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests) (gnome3 and gnome3-xorg passing)
  • 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/) inapplicable
  • Determined the impact on package closure size (by running nix path-info -S before and after) inapplicable
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @lethalman @jtojnar @hedning @worldofpeace for GNOME 3
cc @rnhmjoj for x11/extra-layouts

@bbjubjub2494 bbjubjub2494 mentioned this pull request Dec 27, 2019
Closed
@ofborg ofborg bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos 8.has: module (update) labels Dec 27, 2019
@bbjubjub2494 bbjubjub2494 changed the title Set xkb config root fix services.xserver.extraLayouts under GNOME 3 Dec 27, 2019
@bbjubjub2494 bbjubjub2494 changed the title fix services.xserver.extraLayouts under GNOME 3 Fix services.xserver.extraLayouts under GNOME 3 Dec 27, 2019
jtojnar
jtojnar reviewed Dec 28, 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.

Looks good.

Introduced in 0.6.0, documented in https://xkbcommon.org/doc/current/group__include-path.html. Perhaps gnome-desktop should handle the other paths as well.

Will libgnomekbd need patching as well? Or is this patch sufficient for previews to work in control center?

pkgs/desktops/gnome-3/core/gnome-desktop/default.nix Outdated Show resolved Hide resolved
@ofborg ofborg bot requested a review from jtojnar December 28, 2019 11:18
@bbjubjub2494
Copy link
Member Author

bbjubjub2494 commented Dec 28, 2019 •
edited

Will libgnomekbd need patching as well? Or is this patch sufficient for previews to work in control center?

The previews work. They do have the inesthetic question mark that was mentioned in an upstream issue though. Wait, I could patch libxklavier against that while we're at it.

EDIT: I'm looking into it. Also I thought I had hit "Comment" on this earlier but apparently didn't.

Screenshot from 2019-12-28 09-58-49

@ofborg ofborg bot requested a review from jtojnar December 28, 2019 12:15
@rnhmjoj
Copy link
Contributor

rnhmjoj commented Dec 28, 2019

Nice work @lourkeur! I have been trying to figure out what the issue was with gnome but I gave up.

@bbjubjub2494
Copy link
Member Author

This appears to work. Pushed in the current PR, lmk if it's improper.
Screenshot from 2019-12-28 14-57-40

jtojnar
jtojnar approved these changes Dec 28, 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.

Perfect. Amazing work.

jtojnar
jtojnar approved these changes Dec 28, 2019
View reviewed changes
Copy link
Contributor

@jtojnar jtojnar left a comment •
edited

Choose a reason for hiding this comment

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

Verified in VM that without this patch, the custom layout does not appear in gnome-control-center. With it, it does:

Screenshot from 2019-12-28 20-32-51

Using the following config:

  services.xserver.extraLayouts.custom = {
    description = "my own custom layout";
    symbolsFile = pkgs.fetchurl {
      url = "https://gitlab.epfl.ch/bettens/nixos-profiles/raw/6750def9e2b5f44276df9538c8cf9b88c0e385ba/segments/custom-layout/symbols/custom";
      hash = "sha256-95VFh6t/jthUde5fuiY+zVFbCExJrWTxLcbBA8X3wv4=";
    };
    languages = [ "en" ];
  };

@jtojnar jtojnar merged commit 332cfab into NixOS:staging-next Dec 28, 2019
@jtojnar
Copy link
Contributor

jtojnar commented Dec 28, 2019

Thanks again.

@jtojnar jtojnar mentioned this pull request Dec 28, 2019
Merged
3 tasks
@rnhmjoj rnhmjoj mentioned this pull request Feb 11, 2020
Closed
@bbjubjub2494 bbjubjub2494 deleted the set_XKB_CONFIG_ROOT branch December 31, 2020 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jtojnar jtojnar jtojnar approved these changes

@worldofpeace worldofpeace Awaiting requested review from worldofpeace

@hedning hedning Awaiting requested review from hedning

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: 1-10 10.rebuild-linux: 101-500
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@bbjubjub2494 @rnhmjoj @jtojnar