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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

obsidian: fix wayland support #284971

Closed
wants to merge 3 commits into from

Conversation

tschai-yim
Copy link

@tschai-yim tschai-yim commented Jan 30, 2024

Description of changes

This PR improves the usability of the obsidian package in Wayland with following things:

pkgs.symlinkJoin {
 name = "obsidian";
 paths = [ pkgs.obsidian ];
 buildInputs = [ pkgs.makeWrapper ];
 postBuild = ''
   wrapProgram $out/bin/obsidian \
     --add-flags "--disable-features=WaylandFractionalScaleV1"
 '';
}

Things done

The changes work on my personal NixOS GNOME system.

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • 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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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
  • Fits CONTRIBUTING.md.

Add a 馃憤 reaction to pull requests you find important.

@DontEatOreo
Copy link
Member

Please rebase and follow NixOS Commit conventions instead of Conventional Commits

Copy link
Contributor

@zaninime zaninime left a comment

Choose a reason for hiding this comment

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

I believe one commit with both changes would be enough, however I don't mind the two.

@ofborg ofborg bot requested a review from zaninime January 30, 2024 12:47
@h7x4
Copy link
Member

h7x4 commented Jan 30, 2024

I believe one commit with both changes would be enough

I think two commits are more correct. Since the changes are unrelated, it fits better with the one commit per logical unit convention.

@alexandru0-dev
Copy link

when this is going to be merged? as it seems to not lacking anything and already approved

@Suyashtnt
Copy link

Suyashtnt commented Feb 4, 2024

I tried to use this (by copy pasting the modified file into my packages directory and then importing the custom package) and it seems to fail with the following error:

image
Running on hyprland and latest nixos-unstable.

EDIT: seems to work when downgrading electron to electron_24

@tschai-yim
Copy link
Author

tschai-yim commented Feb 5, 2024

I tried to use this (by copy pasting the modified file into my packages directory and then importing the custom package) and it seems to fail with the following error:

@Suyashtnt you might wanna test it with the master not nixos-unstable branch. For me both work only with the runtime library and/or the feature flag

@m-bdf m-bdf mentioned this pull request Feb 5, 2024
13 tasks
@alexandru0-dev
Copy link

I tried to use this (by copy pasting the modified file into my packages directory and then importing the custom package) and it seems to fail with the following error:.
EDIT: seems to work when downgrading electron to electron_24

same but i'm stable 23.11. Using electron_24 solves the issues

Copy link
Contributor

@niknetniko niknetniko 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

Fixes NixOS#268490 for Obsidian. Implementation inspired by the Insomnia derivation.
@tschai-yim
Copy link
Author

Can we merge this PR? All users that reported a problem either used the wrong version or are not providing further details to see if it's actually an issue with this PR. For niknetniko and me, this patch works and was already proven to work in other chromium apps.

It was still logging some errors for me when starting obsidian but rebasing to the newest master state fixed all these issues.

@ofborg ofborg bot requested a review from kashw2 February 22, 2024 07:58
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1471

@SuperSandro2000 SuperSandro2000 changed the title Fix Obsidian on Wayland obsidian: Fix wayland support Feb 25, 2024
@SuperSandro2000 SuperSandro2000 changed the title obsidian: Fix wayland support obsidian: fix wayland support Feb 25, 2024
@SuperSandro2000
Copy link
Member

Proper fix will be in #285883

@SuperSandro2000 SuperSandro2000 marked this pull request as draft February 25, 2024 17:18
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.

chromium/electron/element-desktop on wayland fails to find libEGL.so