Skip to content

bitwarden-desktop: 2024.12.1 -> 2025.1.1#374021

Merged
talyz merged 1 commit intoNixOS:masterfrom
Saeverix:update-bitwarden-desktop
Jan 24, 2025
Merged

bitwarden-desktop: 2024.12.1 -> 2025.1.1#374021
talyz merged 1 commit intoNixOS:masterfrom
Saeverix:update-bitwarden-desktop

Conversation

@Saeverix
Copy link
Contributor

@Saeverix Saeverix commented Jan 15, 2025

Diff: bitwarden/clients@desktop-v2024.12.1...desktop-v2025.1.1

Changelog: https://github.com/bitwarden/clients/releases/tag/desktop-v2025.1.1

  • Please also backport

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.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.

@Saeverix
Copy link
Contributor Author

Might make #373177 obsolete

@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 15, 2025
@nix-owners nix-owners bot requested a review from amarshall January 15, 2025 11:19
@nyawox
Copy link
Contributor

nyawox commented Jan 15, 2025

hey not sure if i feel comfortable contributing to nixpkgs yet (after what happened to governance last year), but i'd like to at least share some insights here I've discovered personally when attempting to package the latest version:

  1. libsecrets has been replaced with oo7
  2. electron must be updated to version 33
  3. you can easily fix #347350 by adding the following to preBuild phase:
pushd apps/desktop/desktop_native/proxy
cargo build --bin desktop_proxy --release
popd

copy the result binary in installPhase:

cp -r apps/desktop/desktop_native/target/release/desktop_proxy $out/bin

also modify .mozilla/native-messaging-hosts/com.8bit.bitwarden.json to refer to ${pkgs.bitwarden-desktop}/bin/desktop_proxy.

  1. at least until recent nightly build, npmConfigHook failed without setting ESBUILD_BINARY_PATH. even then, I couldn't figure out how to build since both Angular and Vite depend on different esbuild versions. i'm not familiar with js/ts ecosystem and this can be a caching issue or smth that has been fixed on stable release, idk. haven't tried to upgrade to stable release yet. i'm running pre angular update revision
  2. biometric.unix.main.ts has been renamed to os-biometrics-linux.service.ts and patches, installPhase must be updated

@Saeverix
Copy link
Contributor Author

Saeverix commented Jan 15, 2025

@nyawox thank you for the tips. I see that this is not a simple update then. Currently the update is even impossible, since electron_33 isn't even available yet on NixOS. Guess I will have to put this one in the fridge for now.

Looks like it does build fine with electron_33 and oo7. The app functions like expected and I was able to login and use my passwords just fine.
image

As for the other two issues you mentions, I guess that might be something for a separate pull request.

@Saeverix Saeverix marked this pull request as ready for review January 15, 2025 12:48
@nyawox
Copy link
Contributor

nyawox commented Jan 15, 2025

oh yea my bad, since os-biometrics-linux.service.ts rename was merged to nightly https://github.com/bitwarden/clients/tree/main/apps/desktop/src/key-management/biometrics, i automatically assumed it will make to stable release.

i guess desktop_proxy fix is better in a follow up pr?

@Saeverix
Copy link
Contributor Author

Saeverix commented Jan 15, 2025

Looks like it's still biometric.unix.main.ts in https://github.com/bitwarden/clients/tree/desktop-v2025.1.0/apps/desktop/src/key-management/biometrics, so we are good on that.

I think it's best to let #347350 and #371479 be fixed in their own separate pull requests, since it's not really related to the update that I am doing. @nyawox it might be useful if you post your mentioned solutions in both issues.

@nyawox
Copy link
Contributor

nyawox commented Jan 15, 2025

opened a separate pr 👍🏻

edit: damn 371479 was a completely unrelated issue, that's what three hours of sleep do to a person
system biometrics itself (not the browser one) is working for me btw

@Saeverix
Copy link
Contributor Author

Saeverix commented Jan 18, 2025

I suspect @amarshall is not very active recently. Since the linked PR #373177 is open with no response of him for a week. Maybe @ddogfoodd can check this out, since you also participated in #373177?

@ddogfoodd
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374021


x86_64-linux

✅ 1 package built:
  • bitwarden-desktop

@ddogfoodd
Copy link
Contributor

Builds and works fine for me on x86_64-linux

Copy link
Member

@amarshall amarshall left a comment

Choose a reason for hiding this comment

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

Thanks! Just one comment.

@amarshall
Copy link
Member

Also desktop-2025.1.1 is out, but the changes are pretty irrelevant for us right now.

@Saeverix Saeverix changed the title bitwarden-desktop: 2024.12.0 -> 2025.1.0 bitwarden-desktop: 2024.12.0 -> 2025.1.1 Jan 18, 2025
@Saeverix
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374021


x86_64-linux

✅ 1 package built:
  • bitwarden-desktop

@Saeverix
Copy link
Contributor Author

@amarshall and @ddogfoodd I have resolved the comment. I also updated to 2025.1.1 while we are at it. Rebuilt successful without oo7, glib and gtk3, so I removed buildInputs = []; completely.

@ddogfoodd
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374021


x86_64-linux

✅ 1 package built:
  • bitwarden-desktop

@Saeverix Saeverix changed the title bitwarden-desktop: 2024.12.0 -> 2025.1.1 bitwarden-desktop: 2024.12.1 -> 2025.1.1 Jan 19, 2025
@Saeverix
Copy link
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 374021


x86_64-linux

✅ 1 package built:
  • bitwarden-desktop

@Saeverix
Copy link
Contributor Author

I had some unnecessary merge conflicts to resolve, because #373177 was merged by @ck3d even though I mention this PR in there. But anyway, branch is up to date again and this one can be merged now. Also make sure to add the backport label after merging.

@Saeverix
Copy link
Contributor Author

@amarshall a friendly reminder

Copy link
Member

@amarshall amarshall left a comment

Choose a reason for hiding this comment

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

Built on x86_64-linux and performed usual manual tests. Thanks!

@Saeverix
Copy link
Contributor Author

Thanks @amarshall, I assume you are also the one to merge the branch? Since I don't have the required permssion. Or should someone else merge the branch?

@wegank wegank added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jan 23, 2025
Copy link
Contributor

@talyz talyz left a comment

Choose a reason for hiding this comment

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

Works well for me. Tested by running nix run github:Saeverix/nixpkgs/update-bitwarden-desktop#bitwarden-desktop on x86_64-linux.

Thanks!

@talyz talyz merged commit d3c97a6 into NixOS:master Jan 24, 2025
44 of 46 checks passed
@nixpkgs-ci
Copy link
Contributor

nixpkgs-ci bot commented Jan 24, 2025

Backport failed for release-24.11, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin release-24.11
git worktree add -d .worktree/backport-374021-to-release-24.11 origin/release-24.11
cd .worktree/backport-374021-to-release-24.11
git switch --create backport-374021-to-release-24.11
git cherry-pick -x 23c1d61b2b23d574805bfecebe8221eb561e2053

@Saeverix
Copy link
Contributor Author

@talyz (and or @ddogfoodd) I see the automated backport failed for some reason. Is that something I can fix, or should one of you do that since I don't have the required permissions?
If I can help, then I would please have some instructions on how to do it right.

@ddogfoodd
Copy link
Contributor

@talyz (and or @ddogfoodd) I see the automated backport failed for some reason. Is that something I can fix, or should one of you do that since I don't have the required permissions? If I can help, then I would please have some instructions on how to do it right.

Unfortunately I also don't have a clue about backporting yet

@amarshall
Copy link
Member

Backports usually fail due to conflicts. In this case, previous updates were not backported, so it conflicts. I manually created the backport PR: #376545

@Saeverix Saeverix deleted the update-bitwarden-desktop branch January 28, 2025 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 3+ This PR was reviewed and approved by three or more persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants