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

espanso: 0.7.3 -> 2.1.8 #231062

Merged
merged 3 commits into from
May 21, 2023
Merged

Conversation

bobvanderlinden
Copy link
Member

@bobvanderlinden bobvanderlinden commented May 10, 2023

Description of changes

Follow-up from #208949

ZHF: #230712

This is a major update of espanso, with breaking(but fixable) changes.

The version that is currently in Nixpkgs (0.7.3) fails to build.

I've incorporated the feedback from #208949 and updated to the latest nixpkgs-unstable.

There is a git-based dependency in Cargo.lock, so I needed to copy the Cargo.lock as described in https://github.com/NixOS/nixpkgs/blob/master/doc/languages-frameworks/rust.section.md#buildrustpackage-compiling-rust-applications-with-cargo-compiling-rust-applications-with-cargo

I also added a x11Support option. By default both wayland and x11 are supported, but support may be disabled if one wants a smaller package. It should now work for both x11 and wayland users out-of-the-box. Unfortunately espanso always needs libx11, even if NO_X11=true is set.

Things done
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@github-actions github-actions bot added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation labels May 10, 2023
@bobvanderlinden bobvanderlinden force-pushed the espanso-update-2.1.8 branch 2 times, most recently from e477054 to 454100a Compare May 10, 2023 12:22
@ofborg ofborg bot requested a review from pyrox0 May 10, 2023 12:56
@bobvanderlinden bobvanderlinden mentioned this pull request May 10, 2023
12 tasks
@pyrox0
Copy link
Member

pyrox0 commented May 10, 2023

RE: you requesting a review from someone,

I'll try to review it in the next few days if I have some time.

@dit7ya
Copy link
Member

dit7ya commented May 10, 2023

Result of nixpkgs-review pr 231062 run on x86_64-linux 1

2 packages blacklisted:
  • nixos-install-tools
  • tests.nixos-functions.nixos-test
1 package built:
  • espanso

@dit7ya
Copy link
Member

dit7ya commented May 10, 2023

What is the minimal self-contained way to test this package?

I tried

[nix-shell:~/.cache/nixpkgs-review/pr-231062]$ ./results/espanso/bin/.espanso-wrapped service register

But nothing seems to happen when I type :espanso anywhere.

@bobvanderlinden
Copy link
Member Author

bobvanderlinden commented May 10, 2023

I've added a version test to check whether the executable at least works.

To test its functionality I did the following:

$ cat ~/.config/espanso/default.yml
matches:
  - trigger: ":espanso"
    replace: "Hi there!"
$ espanso service start --unmanaged
$ espanso cmd search
# A search window shows up with `Hi there!`
# Hitting enter will type `Hi there!` in the active window.

After that typing :espanso in my browser will replace the text with Hi there!.

@bobvanderlinden
Copy link
Member Author

Hmm, the build of darwin failed. My OSX laptop is broken at the moment, so I have limited ability to test it on OSX. I'll try to make the build work using OfBorg.

@dit7ya
Copy link
Member

dit7ya commented May 10, 2023

Hmmm, this does not work for me, neither the replacement thingy. I wonder how to debug this with more logs?

$ espanso cmd search
# A search window shows up with `Hi there!`
# Hitting enter will type `Hi there!` in the active window.

@bobvanderlinden
Copy link
Member Author

I'm trying this on X11, maybe it isn't working yet on wayland?

You can check the logs using espanso log.
Also, just found that pkill espanso; rm -rf ~/.config/espanso; espanso service start --unmanaged will recreate the configuration that already includes :espanso and Hi there!.

@risicle
Copy link
Contributor

risicle commented May 10, 2023

See if I can trick github into behaving...

@risicle risicle closed this May 10, 2023
@risicle risicle reopened this May 10, 2023
@bobvanderlinden
Copy link
Member Author

I experimented with weston within X11 to determine what is going on with the Wayland build. The NO_X11 envvar was defunct because cargo-make was not used within the build. Cargo is used directly, so I have made sure buildFeatures follows the parameters that the cargo-make config (Makefile.toml) uses.

espanso does not support building with support for both X11 and Wayland as I presumed it did before (due to NO_X11 still working for X11 earlier 😅). So I removed the x11Support option. I made the default to not use wayland, as that is the default for espanso.

Should there be an espanso-wayland that overrides waylandSupport = true? I wasn't able to find conventions for Wayland support in nixpkgs.

@risicle
Copy link
Contributor

risicle commented May 10, 2023

error: hash mismatch in fixed-output derivation '/nix/store/v77jfpr4nvrh8ldp8llsvmimdbh7scn0-py-sr25519-bindings-unstable-2023-03-15-vendor.tar.gz.drv':
         specified: sha256-MwJr6D1akKc+ImCTn4fj8fRuL+PdCmv39usMmcEvPa0=
            got:    sha256-7fDlEYWOiRVpG3q0n3ZSS1dfNCOh0/4pX/PbcDBvoMI=

(damnit too many tabs)

@AngryAnt
Copy link
Member

Should there be an espanso-wayland that overrides waylandSupport = true? I wasn't able to find conventions for Wayland support in nixpkgs.

Going with espanso-wayland offering the override would follow an existing trend - whether or not it is convention:
https://search.nixos.org/packages?channel=unstable&from=0&size=50&sort=relevance&type=packages&query=-wayland
This list used to also include firefox-wayland for example.

@mweinelt
Copy link
Member

error: hash mismatch in fixed-output derivation '/nix/store/v77jfpr4nvrh8ldp8llsvmimdbh7scn0-py-sr25519-bindings-unstable-2023-03-15-vendor.tar.gz.drv':
         specified: sha256-MwJr6D1akKc+ImCTn4fj8fRuL+PdCmv39usMmcEvPa0=
            got:    sha256-7fDlEYWOiRVpG3q0n3ZSS1dfNCOh0/4pX/PbcDBvoMI=

Looks like this was intended for #231103

@RaitoBezarius
Copy link
Member

Is it ready for another review?

@RaitoBezarius RaitoBezarius marked this pull request as draft May 14, 2023 14:52
@bobvanderlinden
Copy link
Member Author

Is it ready for another review?

I'm still trying to get this working on darwin using @NixOS/ofborg 🤞 it works this time.

Other than that, content-wise this is ready for review.

I'll clean up the commits when the darwin build succeeds on ofborg.

@bobvanderlinden bobvanderlinden marked this pull request as ready for review May 14, 2023 17:29
@RaitoBezarius
Copy link
Member

Is it ready for another review?

I'm still trying to get this working on darwin using @NixOS/ofborg crossed_fingers it works this time.

Other than that, content-wise this is ready for review.

I'll clean up the commits when the darwin build succeeds on ofborg.

Running nixpkgs-review on Darwin for you.

@RaitoBezarius
Copy link
Member

This does not eval at all on Darwin:


error:
       … while evaluating call site

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/pkgs/top-level/all-packages.nix:4735:13:

         4734|
         4735|   espanso = callPackage ../applications/office/espanso {
             |             ^
         4736|     inherit (darwin.apple_sdk.frameworks) AppKit Cocoa Foundation IOKit Kernel AVFoundation Carbon QTKit AVKit WebKit System;

       … while calling 'callPackageWith'

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/lib/customisation.nix:128:35:

          127|   */
          128|   callPackageWith = autoArgs: fn: args:
             |                                   ^
          129|     let

       … while evaluating call site

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/lib/customisation.nix:141:10:

          140|         # Filter out arguments that have a default value
          141|         (lib.filterAttrs (name: value: ! value)
             |          ^
          142|         # Filter out arguments that would be passed

       … while calling 'filterAttrs'

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/lib/attrsets.nix:305:5:

          304|     # The attribute set to filter
          305|     set:
             |     ^
          306|     listToAttrs (concatMap (name: let v = set.${name}; in if pred name v then [(nameValuePair name v)] else []) (attrNames set));

       … while evaluating call site

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/lib/customisation.nix:131:15:

          130|       f = if lib.isFunction fn then fn else import fn;
          131|       fargs = lib.functionArgs f;
             |               ^
          132|

       … while calling 'functionArgs'

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/lib/trivial.nix:440:18:

          439|   */
          440|   functionArgs = f:
             |                  ^
          441|     if f ? __functor

       error: undefined variable 'System'

       at /home/raito/.cache/nixpkgs-review/pr-231062/nixpkgs/pkgs/applications/office/espanso/default.nix:94:5:

           93|     WebKit
           94|     System
             |     ^
           95|   ] ++ lib.optionals waylandSupport [
https://github.com/NixOS/nixpkgs/pull/231062 failed to build
$ git worktree prune

@bobvanderlinden
Copy link
Member Author

Sorry, I've fixed the issue. I'm blindly handling darwin support using the builds/buildlogs of OfBorg, so this is quite a slow process for me. Any suggestions on how to do this without a Macbook?

Maybe the darwin platform should be removed for espanso for now until someone picks up the work for MacOS again.

@RaitoBezarius
Copy link
Member

Sorry, I've fixed the issue. I'm blindly handling darwin support using the builds/buildlogs of OfBorg, so this is quite a slow process for me. Any suggestions on how to do this without a Macbook?

Maybe the darwin platform should be removed for espanso for now until someone picks up the work for MacOS again.

Unfortunately, I have an access to a Darwin remote builder for this type of work, but this is not easy to do.
You still seem to have a evaluation failure in the ofborg-eval job regarding System.

@RaitoBezarius
Copy link
Member

Result of nixpkgs-review pr 231062 run on aarch64-darwin 1

1 package built:
  • espanso

@knl
Copy link
Contributor

knl commented May 15, 2023

@bobvanderlinden I can help with darwin builds.

This patch makes it buildable on darwin: knl@4dfe7e3 -- espanso-modulo/build.rs is insidious and it fetches wxgtk to build on macos. I had to patch it to use nixpkgs provided wxgtk.

@knl
Copy link
Contributor

knl commented May 15, 2023

@bobvanderlinden I can help with darwin builds.

This patch makes it buildable on darwin: knl@4dfe7e3 -- espanso-modulo/build.rs is insidious and it fetches wxgtk to build on macos. I had to patch it to use nixpkgs provided wxgtk.

I spoke a bit too soon. This gives an executable on darwin, but the Application crashes when ran from Finder. I'm looking at to how to fix the crash.

@knl
Copy link
Contributor

knl commented May 15, 2023

Made some progress here: knl@5fda294. However, it still doesn't work out of the box. When I start the application, I'm prompted by the "Enable Accessibility" prompt. However, the entry there is not "Espanso", but "bash". Once I enable it for bash, the prompt disappears, however, the daemon doesn't work. If I instead manually start result/bin/espanso start, I get Espanso under Accessibility part of "Privacy & Security" tab.

I don't know how to go around this issue, suggestions welcome :)

@bobvanderlinden
Copy link
Member Author

bobvanderlinden commented May 15, 2023

Woow, nice work! 👍 Looks like a good step forward. I guess the wrapping bash script is what is given permissions and not the application itself. Is the wrapping bash script needed for OSX?

Could it be related to espanso/espanso#1375?


I've cherry-picked your first commit. Though I wasn't sure about the second one. Wrapping the executable for runtime libraries seems like something is wrong with the search paths during the build. Is it not able to find libraries at runtime?

@RaitoBezarius
Copy link
Member

Result of nixpkgs-review pr 231062 run on x86_64-linux 1

1 package blacklisted:
  • nixos-install-tools
2 packages built:
  • espanso
  • espanso-wayland

@RaitoBezarius
Copy link
Member

Result of nixpkgs-review pr 231062 run on aarch64-darwin 1

1 package built:
  • espanso

1 similar comment
@RaitoBezarius

This comment was marked as duplicate.

@RaitoBezarius
Copy link
Member

Result of nixpkgs-review pr 231062 run on x86_64-darwin 1

1 package built:
  • espanso

@kimat
Copy link
Contributor

kimat commented May 31, 2023

This PR also removes openssl_1_1 for espanso as required in #210452 (#210452 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: changelog 8.has: documentation 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.