Skip to content

Vscode/vscodium add use wayland#147945

Closed
TilCreator wants to merge 2 commits intoNixOS:masterfrom
TilCreator:vscode/vscodium-add-useWayland
Closed

Vscode/vscodium add use wayland#147945
TilCreator wants to merge 2 commits intoNixOS:masterfrom
TilCreator:vscode/vscodium-add-useWayland

Conversation

@TilCreator
Copy link
Contributor

@TilCreator TilCreator commented Nov 29, 2021

Motivation for this change

Wanting to use Vscode/Vscodium in Sway without Xwayland.

Things done

Add a useWayland option inspired by bcad474

  • 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/)
  • 21.11 Release Notes (or backporting 21.05 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.

@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Nov 29, 2021
@Synthetica9
Copy link
Member

Synthetica9 commented Nov 30, 2021

In my personal configuration I do

      wrapProgram $out/bin/codium --add-flags \
        "--enable-features=UseOzonePlatform --ozone-platform=wayland"

which Is a bit nicer. This should also be used in nixosTests.vscodium.wayland

@TilCreator TilCreator force-pushed the vscode/vscodium-add-useWayland branch from fde9003 to e0967a7 Compare November 30, 2021 00:35
@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Nov 30, 2021
Synthetica9
Synthetica9 approved these changes Nov 30, 2021
View reviewed changes
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.

lgtm but I'd very much like a second opinion on this.

turion
turion reviewed Nov 30, 2021
View reviewed changes
pkgs/applications/editors/vscode/vscodium.nix
@@ -1,4 +1,4 @@
{ lib, stdenv, callPackage, fetchurl, nixosTests }:
{ lib, stdenv, callPackage, fetchurl, nixosTests, useWayland ? false}:
Copy link
Contributor

@turion turion Nov 30, 2021

Choose a reason for hiding this comment

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

Suggested change
{ lib, stdenv, callPackage, fetchurl, nixosTests, useWayland ? false}:
{ lib, stdenv, callPackage, fetchurl, nixosTests, useWayland ? false }:

@turion
Copy link
Contributor

turion commented Nov 30, 2021

This is great, but maybe it should be documented somewhere so Wayland users will discover it?

@SuperSamus
Copy link
Contributor

SuperSamus commented Nov 30, 2021

Doesn't this conflict with #147557?

@turion
Copy link
Contributor

turion commented Nov 30, 2021

Doesn't this conflict with #147557?

They both solve the same goal, and we probably only need one of the two. Which one it should be is probably a matter of discussion between Wayland users, @TilCreator @wmertens ?

@TilCreator TilCreator mentioned this pull request Nov 30, 2021
Merged
12 tasks
@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 30, 2021

Doesn't this conflict with #147557?

I am using wayland more by accident since plasma switched to it by accident and then kept wayland. I think it is very nice that I can just log into x11 and wayland with the same package set without a rebuild. Thats why I am in very strong favor of fixing this globally via the above quoted PR instead of on a per package basis.

@TilCreator
Copy link
Contributor Author

TilCreator commented Jan 8, 2022

I prefer #147557. Should I close the pr?

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

Reviewers

@turion turion turion left review comments

@bobby285271 bobby285271 Awaiting requested review from bobby285271

@eadwu eadwu Awaiting requested review from eadwu

@amaxine amaxine Awaiting requested review from amaxine

+1 more reviewer

@Synthetica9 Synthetica9 Synthetica9 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@TilCreator @Synthetica9 @turion @SuperSamus @SuperSandro2000