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

bitwarden-cli: init at 1.6.0 #51637

Closed
wants to merge 2 commits into from
Closed

bitwarden-cli: init at 1.6.0 #51637

wants to merge 2 commits into from

Conversation

nlvw
Copy link

@nlvw nlvw commented Dec 6, 2018

Motivation for this change

Make the bitwarden cli client available to nix/nixos. #51212

Things done
  • added "pkgs/tools/security/bitwarden-cli/default.nix"
  • updated all-packages.nix to include bitwarden-cli
  • added myself to the maintainer list as "wolfereign"
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Copy link
Contributor

@srhb srhb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's also some stray trailing whitespace that would be nice to have fixed, but the most critical issue is the failure to build under sandbox. That needs fixing. :)

pkgs/tools/security/bitwarden-cli/default.nix Outdated Show resolved Hide resolved
pkgs/tools/security/bitwarden-cli/default.nix Show resolved Hide resolved
cd "$sourceRoot"
export HOME="."

npm install
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work with sandbox enabled:

❯ nix-build . -A bitwarden-cli
these derivations will be built:
  /nix/store/fl04p4fz7x2nv4xb4cxqlqzrmikgx6ky-bitwarden-cli-v1.6.0.drv
building '/nix/store/fl04p4fz7x2nv4xb4cxqlqzrmikgx6ky-bitwarden-cli-v1.6.0.drv'...
unpacking source archive /nix/store/db36n4ifdw3qlygqaj9z8b3v1ifnhs0l-source
source root is source
npm ERR! cb() never called!

npm ERR! This is an error with npm itself. Please report this error at:
npm ERR!     <https://npm.community>

npm ERR! A complete log of this run can be found in:
npm ERR!     /build/source/.npm/_logs/2018-12-07T10_35_36_167Z-debug.log
builder for '/nix/store/fl04p4fz7x2nv4xb4cxqlqzrmikgx6ky-bitwarden-cli-v1.6.0.drv' failed with exit code 1
error: build of '/nix/store/fl04p4fz7x2nv4xb4cxqlqzrmikgx6ky-bitwarden-cli-v1.6.0.drv' failed

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

npm is used to bring in node dependencies (npm install). The wiki states that sandbox mode removes internet access, so this will not work.

Will this prevent this package being able to merge?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. You’ll need to use Nixpkgs’s node infrastructure to pull in the dependencies purely. Have a look at other node packages to get an idea of how to do this. The manual should help too.

@nlvw
Copy link
Author

nlvw commented Dec 10, 2018

I'm going to go ahead and close this pull request. This is due to Node not being allowed to manage its own native dependencies in the required sandbox mode. Node2Nix looks to be overly complex and I cannot see the reason to not let advanced programming languages like node, rust, and python manage their own native dependencies.

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

5 participants