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

notmuch: add vim plugin #104407

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

notmuch: add vim plugin #104407

wants to merge 4 commits into from

Conversation

Julow
Copy link
Contributor

@Julow Julow commented Nov 20, 2020

Hi !

This PR adds notmuch's Vim plugin and Ruby bindings. The Ruby bindings are required by the Vim plugin.
The first commit is a bit unrelated, it removes emacs from the build dependencies when withEmacs is false. I don't even want this on my computer ! (kidding, it was just an easy fix)

The Vim plugin is enabled by default because it consists only of small static files. The ruby bindings are not big either and ruby is already added as a dependency. Also emacs is enabled by default, so it's fair.

The way I hook the ruby bindings doesn't seem right. I couldn't find examples in nixpkgs and I think bundlerEnv cannot be used here. Is here a better way ?

The ruby bindings have a dependency on the gem mail but it is optional and loaded at runtime, so I haven't included it.

There is a related issue: #43965

Motivation for this change

Make the Vim plugin available from nixpkgs and to use it like this:

let
  notmuch = (pkgs.notmuch.override {
    withEmacs = false;
  }).overrideAttrs (o: {
    doCheck = false; # Too slow
  });

  # Shadows vim, loading the notmuch plugin and a local vimrc
  notmuch_vim = { vimrc }:
    pkgs.runCommand "notmuch-wrapped-vim" {
      buildInputs = [ notmuch ];
    } ''
      # No absolute path to vim on purpose
      mkdir -p "$out/bin"
      cat <<EOF > "$out/bin/vim"
      #!/usr/bin/env bash
      this="$out"
      PATH=\''${PATH//\$this/} # Remove this package from the PATH
      exec vim -c "set packpath+=\$this/vim" -c "packadd notmuch" -c "source ${vimrc}" "\$@"
      EOF
      chmod +x "$out/bin/vim"
      plugin_out="$out/vim/pack/plugins/opt" # Vim plugins directory structure
      mkdir -p "$plugin_out"
      ln -sf "${notmuch}/share/vim" "$plugin_out/notmuch"
    '';

I got a bit creative here, I'll probably use something saner :D

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
  • 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 nixpkgs-review --run "nixpkgs-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.

Install the vim plugin to $out/share/vim.
It is only static files and doesn't add any dependency so it is enabled
by default. Add a `withVim` argument to override this.
They are required by the vim plugin.
Use a setup-hook to set RUBYLIB. This seems like a hacky way.
@vikanezrimaya
Copy link
Member

In case I do want to use the "mail" gem, how to enable it?

@SuperSandro2000
Copy link
Member

Can you update the title to something like notmuch: add vim plugin?

@Julow Julow changed the title Notmuch vim notmuch: add vim plugin Nov 21, 2020
@Julow
Copy link
Contributor Author

Julow commented Nov 21, 2020

Sorry for the broken title.

@kisik21 The mail gem is in nixpkgs already: pkgs.rubyPackages.mail and can be added to your user env or to a shell's buildInputs.

@vikanezrimaya
Copy link
Member

Oh, so vim's ruby support just takes packages from the environment? Sounds impure but can be taken care of with a wrapper, so not a bug per se, just an inconvenience (or a convenient feature, depending on your point of view)

@Julow
Copy link
Contributor Author

Julow commented Nov 21, 2020

It's searching in paths (RUBYLIB) so indeed it's impure but wrapping solves that entirely.

@SuperSandro2000
Copy link
Member

This is a semi-automatic executed nixpkgs-review which is checked by a human on a best effort basis and does not build all packages (e.g. lumo, tensorflow or pytorch).
If you have any questions or problems please reach out to SuperSandro2000 on IRC.

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

1 package failed to build and are new build failure:
12 packages built:
  • aerc
  • afew
  • lieer
  • muchsync
  • neomutt
  • notmuch
  • notmuch-addrlookup
  • python27Packages.notmuch
  • python37Packages.notmuch
  • python37Packages.notmuch2
  • python38Packages.notmuch
  • python38Packages.notmuch2

@stale
Copy link

stale bot commented Aug 4, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Aug 4, 2021
@@ -90,6 +91,8 @@ stdenv.mkDerivation rec {

postInstall = stdenv.lib.optionalString withEmacs ''
moveToOutput bin/notmuch-emacs-mua $emacs
'' + optionalString withVim ''
make -C vim DESTDIR="$out/share/vim" prefix="" install
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this path working for you? I am fairly sure that it'd be safer to do the following:

make -C vim DESTDIR="$out/share/vim-plugins/notmuch" prefix="" install

Also for Neovim users, it'd be nice if you'd also add here:

mkdir -p $out/share/nvim
ln -s $out/share/vim-plugins/notmuch $out/share/nvim/site

@@ -93,6 +93,16 @@ stdenv.mkDerivation rec {
moveToOutput bin/notmuch-emacs-mua $emacs
'' + optionalString withVim ''
make -C vim DESTDIR="$out/share/vim" prefix="" install
'' + optionalString (!isNull ruby) ''
Copy link
Contributor

Choose a reason for hiding this comment

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

How about adding a bit more discoverable argument such as withRuby?

make -C bindings/ruby install DESTDIR="" prefix="$out" install
# This is very specific, see pkgs/development/interpreters/ruby/default.nix
mkdir -p $out/nix-support
cat > $out/nix-support/setup-hook <<EOF
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain a bit more how does this work? Perhaps you can alternatively patch notmuch.vim to use a ruby interpreter that has the bindings from bindings/ruby.

@stale stale bot removed 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md labels Jun 9, 2023
@wegank wegank added 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 2.status: merge conflict labels Mar 19, 2024
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 20, 2024
@wegank wegank marked this pull request as draft March 25, 2024 16:15
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.

None yet

6 participants