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

vimPlugins.vim-go: set g:go_bin_path to work with pure nix-shell #86626

Closed
wants to merge 1 commit into from

Conversation

kalbasit
Copy link
Member

@kalbasit kalbasit commented May 3, 2020

Motivation for this change

Note My patch upstream works for Linux but will definitely break Windows support. I'm not sure we have any Windows use case for this.

Test NeoVim
nix-shell -I nixpkgs=. \
	--pure \
	-p which \
	-p 'with import <nixpkgs> {}; neovim.override { configure = { vam.pluginDictionaries = [ "vim-go" ]; }; }' \
		--run 'cd /path/to/project; export GOPATH=/path/to/gopath; vim main.go'
Test Vim
nix-shell -I nixpkgs=. \
	--pure \
	-p which \
	-p 'with import <nixpkgs> {}; vim_configurable.customize { name = "vim"; vimrcConfig.packages.myVimPackage = with pkgs.vimPlugins; { start = [ vim-go ]; }; }' \
		--run 'cd /path/to/project; export GOPATH=/path/to/gopath; vim main.go'
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • 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.

@kalbasit kalbasit requested a review from jonringer as a code owner May 3, 2020 04:46
@kalbasit kalbasit changed the title vimPlugins.vim-go: set g:go_bin_path to work with pure nix-shell [WIP] vimPlugins.vim-go: set g:go_bin_path to work with pure nix-shell May 3, 2020
@kalbasit kalbasit changed the title [WIP] vimPlugins.vim-go: set g:go_bin_path to work with pure nix-shell vimPlugins.vim-go: set g:go_bin_path to work with pure nix-shell May 3, 2020
@kalbasit kalbasit marked this pull request as draft May 3, 2020 17:24
- It requires gopls, but it's missing because it's no longer provided by gotools.
- It was relying on PATH to find the Go compiler. I've made an update to
  the plugin so it relies on g:go_bin_path when calling system() or when
  starting a job. I've filed a pull request upstream to fix this:
  fatih/vim-go#2876
@kalbasit kalbasit marked this pull request as ready for review July 3, 2020 05:40
@kalbasit
Copy link
Member Author

kalbasit commented Jul 3, 2020

The pull request was not accepted upstream, but I still believe that this is the right thing to do as far as the plugin goes. @jonringer what do you think?

@stale
Copy link

stale bot commented Dec 31, 2020

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 Dec 31, 2020
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 21, 2021
@SuperSandro2000
Copy link
Member

I am not sure this is the right approach and the patch probably breaks often.

@kalbasit
Copy link
Member Author

I am not sure this is the right approach and the patch probably breaks often.

I'm concerned the same. I intend to go back upstream with a different less-intrusive patch to allow it to be pure. If it was not accepted and we care about its purity then I don't see another approach :|

@jonringer
Copy link
Contributor

also might be possible to use propagatedUserEnvPkgs to export go. But that's more of a hack, and may have surprising results if it replaces an already present go package.

@stale
Copy link

stale bot commented Sep 19, 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 Sep 19, 2021
@teto
Copy link
Member

teto commented Jun 24, 2022

this is too big a deviation from upstream. If setting g:go_bin_path is enough, you could add it to this list I am building up #172538

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 24, 2022
@teto
Copy link
Member

teto commented Jul 23, 2022

can we close this ?

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

4 participants