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

zplugin:init at 2.3 #73764

Open
wants to merge 1 commit into
base: master
from
Open

zplugin:init at 2.3 #73764

wants to merge 1 commit into from

Conversation

@pasqui23
Copy link
Contributor

pasqui23 commented Nov 19, 2019

Motivation for this change
Things done

Building fails with

nix-env -f ./. -iA zplugin
installing 'zplugin-2.3'
these derivations will be built:
  /nix/store/sanj1x0ishjzrqbpkwhpmwr41hl2j6vn-zplugin-2.3.drv
building '/nix/store/sanj1x0ishjzrqbpkwhpmwr41hl2j6vn-zplugin-2.3.drv'...
unpacking sources
unpacking source archive /nix/store/wl0pvhglyvkyvny2hgjvgyn0bi89ilnr-source
source root is source
patching sources
configuring
no configure script, doing nothing
installing
/nix/store/npsin1jlnzs5sqxpsv2mk5gbnm4hk4kr-stdenv-linux/setup: eval: line 1337: unexpected EOF while looking for matching `"'
builder for '/nix/store/sanj1x0ishjzrqbpkwhpmwr41hl2j6vn-zplugin-2.3.drv' failed with exit code 2
error: build of '/nix/store/sanj1x0ishjzrqbpkwhpmwr41hl2j6vn-zplugin-2.3.drv' failed
  • 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 nix-review --run "nix-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.
Notify maintainers

cc @

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 19, 2019

#TODO:doc output

meta = with lib; {
license = licenses.mit;

This comment has been minimized.

Copy link
@jonringer

jonringer Nov 19, 2019

Contributor

license is listed twice

Suggested change
license = licenses.mit;
pkgs/shells/zsh/zplugin/default.nix Show resolved Hide resolved
@pasqui23

This comment has been minimized.

Copy link
Contributor Author

pasqui23 commented Nov 24, 2019

What is the exact procedure for proposing as mantainer?

@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Nov 24, 2019

it's pretty relaxed in nixpkgs, the main qualification is just general interest in the package. Most of the time, maintainers know how to use the software so they can verify that updates to it don't break any workflows.

@little-dude

This comment has been minimized.

Copy link

little-dude commented Jan 7, 2020

@pasqui23 do you plan to push this forward? Otherwise I might try to pick it up.

@pasqui23

This comment has been minimized.

Copy link
Contributor Author

pasqui23 commented Jan 8, 2020

So apparently it builds with the latest code

@pasqui23 pasqui23 changed the title [WIP]zplugin:init at 2.3 zplugin:init at 2.3 Jan 8, 2020
@pasqui23 pasqui23 marked this pull request as ready for review Jan 8, 2020
@pasqui23

This comment has been minimized.

Copy link
Contributor Author

pasqui23 commented Jan 8, 2020

@little-dude yes,I still want to push it

Copy link
Contributor

jonringer left a comment

you could also use install -DmXXX [sources] -t dest to greatly simplify a lot of this installation logic (it will create parent directories for you)

{ stdenvNoCC, lib, fetchFromGitHub }:
stdenvNoCC.mkDerivation rec {
pname = "zplugin";
version = "2.3";

This comment has been minimized.

Copy link
@jonringer

jonringer Jan 8, 2020

Contributor
Suggested change
version = "2.3";
version = "2.3";
@@ -0,0 +1,40 @@
{ stdenvNoCC, lib, fetchFromGitHub }:

This comment has been minimized.

Copy link
@jonringer

jonringer Jan 8, 2020

Contributor
Suggested change
{ stdenvNoCC, lib, fetchFromGitHub }:
{ stdenvNoCC, lib, fetchFromGitHub }:
pkgs/shells/zsh/zplugin/default.nix Outdated Show resolved Hide resolved
pkgs/shells/zsh/zplugin/default.nix Outdated Show resolved Hide resolved
Copy link
Contributor

jonringer left a comment

sorry, forgot to add that installShellCompletion is another "package", so it needs to be added to avoid:

/nix/store/plcn9z5lzjd9lhij1cj73k2nwxkng4rq-stdenv-linux/setup: line 1309: installShellCompletion: command not found
repo = pname;
rev = "v${version}";
sha256 = "0qqv5p19s8jb06d6h55dm4acji9x2rpxb2ni3h7fb0q43iz6y85w";
};

This comment has been minimized.

Copy link
@jonringer

jonringer Jan 9, 2020

Contributor
Suggested change
};
};
nativeBuildInputs = [ installShellCompletion ];
@@ -0,0 +1,39 @@
{ stdenvNoCC, lib, fetchFromGitHub }:

This comment has been minimized.

Copy link
@jonringer

jonringer Jan 9, 2020

Contributor
Suggested change
{ stdenvNoCC, lib, fetchFromGitHub }:
{ stdenvNoCC, lib, fetchFromGitHub, installShellCompletion }:
@jonringer

This comment has been minimized.

Copy link
Contributor

jonringer commented Jan 9, 2020

don't forget to squash the commits, to squash the last 6 commits into the first you can do:

git reset HEAD~5 # put the last 5 changes into unstaged
git add pkgs/
git commit --amend --no-edit # amend into previous commit
git push <fork> <branch> --force

for other incremental changes into the last commit, you can just do:

git add pkgs/
git commit --amend --no-edit # amend into previous commit
git push <fork> <branch> --force
@pasqui23 pasqui23 force-pushed the pasqui23:zplg branch 2 times, most recently from f5c42a7 to 04bf131 Jan 9, 2020
# Zplugin-module files
find zmodules/ -type d -exec install -dm 755 "{}" "$outdir/{}" \;
find zmodules/ -type f -exec install -m 744 "{}" "$outdir/{}" \;

This comment has been minimized.

Copy link
@jonringer

jonringer Jan 9, 2020

Contributor

should this preserve the same file structure?

Suggested change
find zmodules/ -type f -exec install -m 744 "{}" "$outdir/{}" \;
find zmodules/ -type f -exec install -m 744 "{}" "$outdir/zmodules/{}" \;

This comment has been minimized.

Copy link
@pasqui23

pasqui23 Jan 9, 2020

Author Contributor

Honestly now that I look it better,it seem it would be better if packaged separately

@pasqui23 pasqui23 force-pushed the pasqui23:zplg branch from 04bf131 to e949b27 Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.