Update vim_configurable to vim 8.0005

[winksaville]

Motivation for this change

fix #18800

Things done

• Tested using sandboxing
  (nix.useSandbox on NixOS,
  or option build-use-sandbox in nix.conf
  on non-NixOS)
• Built on platform(s)
  • NixOS
  • OS X
  • Linux
• Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Fits CONTRIBUTING.md.

---

[mention-bot]

@winksaville, thanks for your PR! By analyzing the annotation information on this pull request, we identified @kamilchm, @hrdinka and @MarcWeber to be potential reviewers

[groxxda]

Can't we just use the src from the regular vim expression? Otherwise this will diverge eventually

[LnL7]

I was wondering the same thing, this should probably use the vim drv as a starting point.

[groxxda]

Using vim.src is probably not worth the effort since vim needs refactoring anyway ( #18763 )

[winksaville]

I was wondering if vim should just be derived from vim_configurable as
vim_configurable is more general? But, I thought this was the simplest
thing for now as they've been using different versions for awhile.

On Wed, Sep 21, 2016, 5:55 AM Alexander Ried notifications@github.com
wrote:

@groxxda approved this pull request.

Using vim.src is probably not worth the effort since vim needs
refactoring anyway ( #18763
#18763 )

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#18801 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA-hHJ_3odLjyYi2XpG5A0FBu7Ux6oM8ks5qsSk7gaJpZM4KCRjs
.

[LnL7]

This breaks the darwin build, the python patch does not apply.

[winksaville]

@LnL7, when you say "the python patch doesn't apply", what python patch? How can I reproduce the problem? (Note: I'm a newbie so pardon my ignorance).

[LnL7]

@winksaville You can remove the optional call to always apply the patch, I can take a look if you can't figure it out but I'm not sure if I'll have time today.

https://github.com/winksaville/nixpkgs/blob/08d7cfa4203dd5adc8b98979ab085a8a100c3f34/pkgs/applications/editors/vim/configurable.nix#L76

[winksaville]

@LnL7 are you saying we never want to apply the patch or we always want to apply the patch?
Is this a problem with vim 8.005 only or with other builds too? For instance I noticed that vim/configurable.nix was using 7.4.826 where as previously vim/default.nix was using 7.4.1585.

Also, note the odd logic in line 75 with the 'or true':
(stdenv.isDarwin && (config.vim.darwin or true))

I currently don't have a darwin environment setup so I'm unable to test.

[LnL7]

@winksaville I suspect the vim package does not need that patch because it does not have python support

[groxxda]

@winksaville the attr.path or true is common. It is there because config might not have a vim attribute, or if it has, that vim attribute set might not have a darwin attribute. Accessing an invalid key will yield an error, but the or true will "catch" the error and return true instead:

nix-repl> config = { vim = { linux = true; }; }

nix-repl> config.vim.darwin
error: attribute ‘darwin’ missing, at (string):1:1

nix-repl> config.vim.darwin or true
true

@LnL7 was suggesting you remove the optional so when you build vim the patch gets applied.

Another option would be to get the source manually and apply the patch. Or with help of nix:

 ~/nixpkgs nix-shell . -A vim_configurable 

[nix-shell:~/nixpkgs]$ cd /tmp

[nix-shell:/tmp]$ unpackPhase
�[3punpacking source archive /nix/store/yhz9549411wsyfp5anxlclgcb266xvva-vim-v7.4.826-src
�[qsource root is vim-v7.4.826-src

[nix-shell:/tmp]$ cd vim-v7.4.826-src/

[nix-shell:/tmp/vim-v7.4.826-src]$ $prePatch 

[nix-shell:/tmp/vim-v7.4.826-src/src]$ patch < ~/nixpkgs/pkgs/applications/editors/vim/python_framework.patch
patching file configure
[...]

This will most probably fail and you can inspect further. Interestingly the patch did not apply for me with the old vim version either...

[winksaville]

@groxxda, but doen't (config.vim.darwin or true) always evaluate to true and would therefore appear to me to be unnecessary. And regarding removing optional that's trivial but doing much beyond that is going to get me into trouble with you guys.

@LnL7, python can support python as an interpreter although I've never used it, see here and here for instance. So I'm guessing if you install vim using nix is that vim should use nix's version of python not some random version installed on the platform. But as I said, I'm just guessing.

[groxxda]

@winksaville i admit this is not intuitive because the or is quite different from the logic ||:

nix-repl> { x = false; }.x or true
false

nix-repl> { x = false; }.x or false
false

nix-repl> { x = false; }.y or true
true

nix-repl> { x = false; }.y or false
false

while the logic || yields:

nix-repl> { x = false; }.y || true
error: attribute ‘y’ missing, at (string):1:1

nix-repl> { x = false; }.y || false
error: attribute ‘y’ missing, at (string):1:1

The proposal was only to remove it to test the patch locally and fix the patch. Of cource you should not commit the removed optional 😉

[winksaville]

@groxxda, I think I understand or, I see from here its a set operator that defines a default if the Set on the LHS yields an "empty" result or an "error", please correct me if I'm wrong.

@LnL7 please test and let me know if you'd like me to do anything else or if it needs a more sophisticated change I can certainly close this pull request and a proper one can be supplied.

[LnL7]

@winksaville The patch is no longer needed for this version as far as I can tell, you can just remove the python_framework.patch file and the patches attribute.

[winksaville]

OK

On Thu, Sep 22, 2016, 12:20 PM Daiderd Jordan notifications@github.com
wrote:

@winksaville https://github.com/winksaville The patch is no longer
needed for this version as far as I can tell, you can remove the
python_framework.patch file and the patches attribute.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#18801 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA-hHH9Uhz0QwaAxn79Eqrri2-gaCSVYks5qstUOgaJpZM4KCRjs
.

[winksaville]

@LnL7 I've updated the pull request to remove python_framework.patch.

If a user 'manually' changed version to 7.4.826 or some other earlier version, would vim no longer 'work' on darwin? If so should we instead conditionally apply python_framework.patch?

[LnL7]

While you indeed can override the version, this is not something we account for in nixpkgs.
In some cases we do keep multiple versions around, but only if there's a reason for that.