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.

Sign up for GitHub

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

Jump to bottom

bitwarden-cli: 2024.9.0 -> 2024.11.0 #350601

Merged
merged 1 commit into from
Nov 26, 2024
Merged

bitwarden-cli: 2024.9.0 -> 2024.11.0 #350601

merged 1 commit into from
Nov 26, 2024

Conversation

dotlambda
Copy link
Member

@dotlambda dotlambda commented Oct 23, 2024 •
edited
Loading

Diff: bitwarden/clients@cli-v2024.9.0...cli-v2024.11.0

Changelog: https://github.com/bitwarden/clients/releases/tag/cli-v2024.11.0

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@dotlambda dotlambda marked this pull request as draft October 24, 2024 22:39
@amarshall
Copy link
Member

You may be aware (and maybe why you marked draft), but there’s currently some licensing issues with Bitwarden, see bitwarden/clients#11611

@amarshall
Copy link
Member

Licensing issue is resolved in v2024.11.0+

@afh
Copy link
Member

afh commented Nov 19, 2024

@dotlambda Mind changing this PR to update bitwarden-cli to 2024.11.0 released November 14th?

@dotlambda dotlambda changed the title bitwarden-cli: 2024.9.0 -> 2024.10.0 bitwarden-cli: 2024.9.0 -> 2024.11.0 Nov 19, 2024
@dotlambda dotlambda marked this pull request as ready for review November 19, 2024 17:11
@afh
Copy link
Member

afh commented Nov 19, 2024

Result of nixpkgs-review pr 350601 run on aarch64-darwin 1

1 package failed to build:
  • bitwarden-cli

@dotlambda
Copy link
Member Author

See #340138 for the Darwin build.

gepbird
gepbird suggested changes Nov 20, 2024
View reviewed changes
Copy link
Contributor

@gepbird gepbird left a comment

Choose a reason for hiding this comment

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

Builds and tested it on x86_64-linux, other than this LGTM

pkgs/by-name/bi/bitwarden-cli/package.nix Outdated
@@ -28,7 +28,7 @@ buildNpmPackage rec {

nodejs = nodejs_20;

npmDepsHash = "sha256-L7/frKCNlq0xr6T+aSqyEQ44yrIXwcpdU/djrhCJNNk=";
npmDepsHash = "sha256-YzhCyNMvfXGmgOpl3qWj1Pqd1hY8CJ9QLwQds5ZMnqg=";

nativeBuildInputs = [
(python3.withPackages (ps: with ps; [ setuptools ]))
Copy link
Contributor

Choose a reason for hiding this comment

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

distutils was required by electron/rebuild#1116, from this release it builds without that and even without python3

Suggested change
(python3.withPackages (ps: with ps; [ setuptools ]))

Copy link
Member Author

Choose a reason for hiding this comment

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

even without python3

Why's that? Do they ship a Python binary?

Copy link
Contributor

@gepbird gepbird Nov 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Looks like they aren't even using node-gyp.

WIth the 2024.11.0 release, putting rm node_modules/@electron/node-gyp/configure.js before the npm rebuild command, bitwarden-cli still builds. Just to make sure doing the same in the older 2024.9.0 version with rm node_modules/node-gyp/configure.js (electron hasn't forked node-gyp in that time) the build fails as expected with it not finding the configure script.

However the strange thing is I can delete the whole node-gyp directory in both 2024.9.0 and 2024.11.0 releases and the build still passes, is it possible that it wasn't even used before?

Copy link
Member

Choose a reason for hiding this comment

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

The npm in nodejs includes node-gyp dep already, so it’s possibly falling-back to that.

@dotlambda
Copy link
Member Author

The Darwin build fails with

npm error ld: warning: dylib (/nix/store/lzzd5jgybnpfj86xkcpnd54xgwc4m457-libcxx-16.0.6/lib/libc++.dylib) was built for newer macOS version (11.0) than being linked (10.7)
npm error ld: warning: object file (Release/obj.target/validation/src/validation.o) was built for newer macOS version (11.0) than being linked (10.7)
npm error fatal error: /nix/store/82ap3f2iqvb2ysjhqkd95k8h9r3y12f6-cctools-1010.6/bin/lipo: /private/tmp/nix-build-bitwarden-cli-2024.11.0.drv-0/validation-f7f5b1/validation-x86_64.out and /private/tmp/nix-build-bitwarden-cli-2024.11.0.drv-0/validation-38c91b/validation-arm64.out have the same architectures (arm64) and can't be in the same fat output file

@dotlambda dotlambda mentioned this pull request Nov 24, 2024
Open
13 tasks
@dotlambda dotlambda merged commit d66e8f8 into NixOS:master Nov 26, 2024
33 of 34 checks passed
@dotlambda dotlambda deleted the bitwarden-cli branch November 26, 2024 23:48
@afh afh mentioned this pull request Nov 27, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gepbird gepbird gepbird requested changes

@amarshall amarshall amarshall left review comments

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dotlambda @amarshall @afh @gepbird