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

fzf: Expose interactive shell scripts #25080

Conversation

binarin
Copy link
Contributor

@binarin binarin commented Apr 21, 2017

Note that while those scripts have completion in their name, they
are not a completion scripts that should be added to
bash-completion.d and friends.

Explicitly listing outputsToInstall is necessary so we can have all
the goodies (vim-plugin, shell scripts) in our ~/.nix-profile when
using nix-env. See
#24717 (comment)

Motivation for this change
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
    • macOS
    • 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.

Note that while those scripts have 'completion' in their name, they
are not a completion scripts that should be added to
'bash-completion.d' and friends.

Explicitly listing `outputsToInstall` is necessary so we can have all
the goodise (vim-plugin, shell scripts) in our `~/.nix-profile` when
using `nix-env`. See
NixOS#24717 (comment)
@mention-bot
Copy link

@binarin, thanks for your PR! By analyzing the history of the files in this pull request, we identified @kamilchm, @sigma and @nhooyr to be potential reviewers.

@dezgeg
Copy link
Contributor

dezgeg commented Apr 21, 2017

Don't install them to out but rather to $bin. Then this outputsToInstall hackery isn't needed.

@binarin
Copy link
Contributor Author

binarin commented Apr 21, 2017

@dezgeg But then it's logical to drop multiple outputs altogether, isn't it?

@dezgeg
Copy link
Contributor

dezgeg commented Apr 22, 2017

No, $out seems to contain stuff like static libraries (share/go/pkg/linux_amd64/github.com/junegunn/fzf/src/algo.a) which isn't needed at runtime. Now if those files aren't needed at all (i.e. the package doesn't provide a go library that other packages can link against) then those should be deleted and only then the extra output can be dropped.

@7c6f434c
Copy link
Member

7c6f434c commented May 1, 2017

If these are static libraries, maybe it is more natural to install them to dev or lib?

@binarin binarin closed this Jul 28, 2017
binarin added a commit to binarin/nixpkgs that referenced this pull request Jul 28, 2017
So that helper scripts can be easily sourced in interactive shell
configuration. `autojump` package was already present and had the same
requirements for findind a `share` folders, so I took an inspiration
there.

I beleive this is a better alternative to:
- NixOS#25080
- NixOS#27058

Replacing `$out/share/shell` with `$bin/share/fzf` was necessary to
prevent dependency loop in produced derivations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants