[PATCH 0/4] vim: Detect plugins via search paths.

Hi!

These patches make Vim use native-search-paths to search for plugins,

instead of manually searching for them in a set of hardcoded paths,

which is how it was done up till now.

I've added the same search paths to Neovim, which unlike Vim, wasn't

auto-detecting Vim plugins at all before.

Neovim also searches XDG_DATA_DIRS for Neovim-specific plugins, so I'd

appreciate if https://issues.guix.gnu.org/48112 was merged too.

Finally, I've updated the package definitions to use the new style with

gexps and label-less inputs.

SeerLite

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 6 ++++++

2 files changed, 10 insertions(+), 7 deletions(-)

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 16 ++++++++++++++--

1 file changed, 14 insertions(+), 2 deletions(-)

labels.

[arguments]: Convert to list of gexps and inline the "guix.vim" gexp.

---

gnu/packages/vim.scm | 127 +++++++++++++++++++++----------------------

1 file changed, 62 insertions(+), 65 deletions(-)

[native-inputs]: Remove "guix.vim" gexp and remove labels.

[arguments]: Convert to list of gexps, inline the "guix.vim" gexp, adjust lua

input names and remove trailing #t's.

---

gnu/packages/vim.scm | 114 +++++++++++++++++++++----------------------

1 file changed, 55 insertions(+), 59 deletions(-)

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

The tzdata in in 'native-inputs', not 'inputs', so this code needs to

look in '(or native-inputs inputs)', not 'inputs', otherwise this code

will fail with a '&search-path' exception from 'search-input-directory'

when cross-compiling (untested).

Greetings,

Maxime.

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

This is test stuff, and these binaries do not seem to be present in

'inputs', they would be in the implicit 'native-inputs', so these would

need to search in '(or native-inputs inputs)' instead of 'inputs' to

avoid &search-path exceptions when cross-compiling:

(substitute* '("src/testdir/...")

(("/bin/sh") (search-input-file (or native-inputs inputs) "bin/sh")))

Or simpler, there's a procedure for looking for 'bin/TOOL' in native-

inputs: 'which'!

;; the original code!

(substitute* '("src/testdir/...")

(("/bin/sh") (which "sh")))

Why the change from 'which' to 'search-input-file'?

Greetings,

Maxime.

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

this can be simplified:

(native-search-paths (package-native-search-paths vim))

Greetings,

Maxime.

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

Nice! That would make the "--pure" option in

"guix shell --pure vim perhaps-some-vim-plugins ... -- vim" actually

work! (untested, I'm just _looking_ at the patches, not actually

testing them)

Greetings,

Maxime.

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

Is "installman.sh" used during the build, or when vim is run?

In the former case, this should look into (or native-inputs

inputs) instead for cross-compilation reasons, and could be simplified

to:

(("/bin/sh") (which "sh"))

Greetings,

Maxime.

SeerLite via Guix-patches via schreef op wo 02-03-2022 om 10:17 [-

0300]:

This stops package transformations from being able to change the

guix.vim used. WDYT of using a variant of 'search-input-file', that

strips the /gnu/store/HASH- part of the store item names and tests

if the remainder equals the string "guix.vim"? Such a procedure does

not currently exist to my knowledge, but it could be written and would

be useful outside the neovim definition.

Greetings,

Maxime.

Hi, thanks for the review!

On 3/2/22 14:29, Maxime Devos wrote:

Whoops, I forgot I made this change.

The blog post that introduces label-less inputs also introduces

'search-input-file', which made me think they were both part of the

"package definition modernization process".

I asked on IRC if that was the case, and although I didn't get a clear

answer for that, someone told me they preferred using

'search-input-file' because it raises an exception when no file is found.

What do you think about that? Should I stick with 'search-input-file' or

is 'which' alright?

It makes sense that I'd have to use native-inputs though. My bad!

On 3/2/22 14:40, Maxime Devos wrote:

Ah, is this also why package inputs are specified explicitly instead of

using gexps?

Is there any benefit to being able to change such a file via a

transformation? Editing the vimrc feels like editing the default

configuration of the package, rather than swapping a dependency. And

that looks odd to me, maybe because I'm more used to package

dependencies as inputs.

The procedure sounds like a good idea, but wouldn't it be easier to just

make `local-file` not prepend the hash to the filename by storing it

inside a parent directory with said hash instead? Is something like that

possible?

Thank you again for the corrections. Should I send the entire patch

series again or only the patches I want to correct? I'm new to the whole

debbugs/email patch workflow.

SeerLite schreef op do 03-03-2022 om 13:49 [-0300]:

The problem with 'which' was that it looks in 'native-inputs' (+ inputs

when compiling natively), even when we need 'inputs'. To fix this,

adding an 'inputs/native-inputs' argument was proposed

(https://issues.guix.gnu.org/47869). That interface was considered

a bit weird, so then (string-append (assoc-ref ...) "bin/...") was

proposed. That's rather verbose though, so 'search-input-file' was

introduced.

In short, 'search-input-file' was introduced to help with locating

binaries from 'inputs' (at least from my POV, shortly after ludo began

using 'search-input-file' for delabelification).

For looking in native-inputs (compilation tools etc.), I would prefer

'which' above the 'search-input-file' because it's a lot more concise,

though I suppose there is an argument to be made for using 'search-

input-file' for consistency.

There's the downside of not raising an exception in case the file is

not found though. However, there appears to be very few code that

actually relies on the 'return #f' behaviour (search for

'(or (which' and '(if (which'), so I think the behaviour can

be changed without much trouble (on 'core-updates').

Greetings,

Maxime.

Hi Seerlite,

SeerLite <seerlite@nixnet.email> skribis:

To avoid confusion, it’s more convenient if you resend the entire patch

series; make sure to use ‘git format-patch --subject-prefix="PATCH v2"’.

Thank you!

Ludo’.

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 6 ++++++

2 files changed, 10 insertions(+), 7 deletions(-)

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 16 ++++++++++++++--

1 file changed, 14 insertions(+), 2 deletions(-)

Hi! Sorry for the delay, I've been very busy.

I sent the rebased patches but only the bits that added search paths. There were some updates to both Vim and Neovim since I sent the first patch and I didn't want to deal with fixing the conflicts and re-understanding the style changes I had even forgotten I made. So I'm sending just the functional parts of the patch and hopefully someone else can update the package styles if necessary :)

Oh and I'd like to remind you of this patch: https://issues.guix.gnu.org/48112 which along with my patch would make it so Neovim can finally use both Vim and Neovim-exclusive plugins. I'd appreciate if it got merged too.

SeerLite

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 6 ++++++

2 files changed, 10 insertions(+), 7 deletions(-)

and remove trailing #t's.

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 17 +++++++++++++----

1 file changed, 13 insertions(+), 4 deletions(-)

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

[phases]: Search for tzdata files in the correct input fields.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 8 +++++++-

2 files changed, 11 insertions(+), 8 deletions(-)

and remove trailing #t's.

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 17 +++++++++++++----

1 file changed, 13 insertions(+), 4 deletions(-)

Rebased on top of 182b25fb (https://issues.guix.gnu.org/55045) and

fixed the wrong lookup for tzdata files in inputs instead of

native-inputs again, as Maxime had pointed out (something I thought was

caused by my previous style/gexp patch when it was not). I wasn't able

to figure out how to test it though.

SeerLite

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

[phases]: Search for tzdata files in the correct input fields.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 8 +++++++-

2 files changed, 11 insertions(+), 8 deletions(-)

and remove trailing #t's.

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 17 +++++++++++++----

1 file changed, 13 insertions(+), 4 deletions(-)

Fixed missing ')'. Whoops.

SeerLite

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

[phases]: Search for tzdata files in the correct input fields.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 10 ++++++++--

2 files changed, 12 insertions(+), 9 deletions(-)

and remove trailing #t's.

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 17 +++++++++++++----

1 file changed, 13 insertions(+), 4 deletions(-)

The last patches were broken. I did not realize I had done a "make

clean" inside my Guix repo and that made ./pre-inst-env guix use my

profile channel instead of the repo. Sorry about that:)

Now both build fine and work correctly. I also tested cross-compilation

(guix build --target=x86_64-linux-gnu and also

--target=aarch64-linux-gnu) but both fail. Note however that that was

also the case before my patches, and I've got no idea how to get it

working myself.

SeerLite

Previously a hardcoded list of directories was used, which albeit quite

accurate during normal use, didn't fully respect the active Guix environment

and didn't make use of Guix's search-paths feature.

specification for 'GUIX_VIMRUNTIME'.

[phases]: Search for tzdata files in the correct input fields.

---

gnu/packages/aux-files/guix.vim | 11 ++++-------

gnu/packages/vim.scm | 10 ++++++++--

2 files changed, 12 insertions(+), 9 deletions(-)

and remove trailing #t's.

[native-search-paths]: Add search path specification for 'GUIX_VIMRUNTIME'.

---

gnu/packages/vim.scm | 17 +++++++++++++----

1 file changed, 13 insertions(+), 4 deletions(-)

Am Mittwoch, dem 22.02.2023 um 17:07 -0300 schrieb SeerLite:

Should be [#:phases]<set-environment-variables>. For clarity's sake, I

think this phase should be renamed to <prepare-test-environment>, so

that it doesn't look as though these variables affect the build output.

Otherwise LGTM, but I'm not a vim person.

Am Mittwoch, dem 22.02.2023 um 17:07 -0300 schrieb SeerLite:

Again, should be [#:phases].

When you need to join sentences with "and" it's typically a sign that

these are unrelated changes. At the very least, you should write out

those sentences. I'm also a proponent of doing stylistic changes in a

separate patch, but since you're already waiting for this a long time,

I'm willing to take this series without that.

Can we inherit this from vim via (package-native-search-paths vim)?

Sadly, this patch currently doesn't apply on master; or at least what I

have for master, which lacks any changes in vim.scm, though. Could you

send one with these points adjusted and --reroll-count=8?

Cheers