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_rs: 1.9.1 -> 1.13.0 #76673

Closed
wants to merge 1 commit into from
Closed

bitwarden_rs: 1.9.1 -> 1.13.0 #76673

wants to merge 1 commit into from

Conversation

dali99
Copy link
Member

@dali99 dali99 commented Dec 30, 2019 •
edited

Motivation for this change

I was running bitwarden_rs in docker, but as I'm switching over to NixOS I wanted to use the module instead.

The old version in nixpkgs meant my db was not compatible. So I made a package and added it to nixpkgs.config.packageOverrides with callPackage.

THE ABOVE IS THE ONLY WAY THIS WAS TESTED. But it seems to work fine with the module.

Things done
  • 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 nixpkgs-review --run "nixpkgs-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 @msteen

@msteen
Copy link
Contributor

msteen commented Dec 30, 2019

The PR #74927 already exist for updating the version of bitwarden_rs, but my feedback would result in almost the same file changes as you propose, so considering the other PR has not been updated to reflect this yet, I don't mind this PR to be merged instead. I have build it on my machine and your changes seem to work as far as I have tested, so it has my OK to be merged.

@dali99
Copy link
Member Author

dali99 commented Dec 30, 2019

Oh wow, I somehow missed that PR in my search!

You mention doCheck not working in your comment, but it seems to me it is now?

@msteen
Copy link
Contributor

msteen commented Dec 30, 2019 •
edited

Yeah, I clearly remember testing it after seeing the nix code behind buildRustPackage not mentioning dontCheck but doCheck instead, but I might have made a mistake in that test. I am trying to reproduce it now (unfortunately building it takes forever), but it was probably a mistake while testing.
Edit: Cannot reproduce it, so it indeed seems to have been a mistake in my test at the time.

@Br1ght0ne
Copy link
Member

Br1ght0ne commented Dec 30, 2019 •
edited

Thanks @msteen for the help and @dali99 for this PR. I added you as co-authors in my PR, and incorporated the changes @msteen proposed. Is that OK?

@bbigras
Copy link
Contributor

bbigras commented Jan 20, 2020

Can we get one of the PR updated to 1.13.1 and merged please?

@msteen msteen mentioned this pull request Jan 27, 2020
Merged
10 tasks
@dali99
Copy link
Member Author

dali99 commented Jan 28, 2020

Closing in favour of #78615

@dali99 dali99 closed this Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@dali99 @msteen @Br1ght0ne @bbigras @Janik-Haag