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

protonmail-bridge: 1.2.3-1 -> 1.5.0 #103037

Merged
merged 1 commit into from
Nov 26, 2020
Merged

protonmail-bridge: 1.2.3-1 -> 1.5.0 #103037

merged 1 commit into from
Nov 26, 2020

Conversation

lightdiscord
Copy link
Member

@lightdiscord lightdiscord commented Nov 6, 2020

Provide a minimal cli only version see #97991.

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

EDIT: I'll rebase and squash once all review are marked as resolved.

Sorry, something went wrong.

@ofborg ofborg bot requested a review from kalbasit November 6, 2020 21:32
@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Nov 6, 2020
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin labels Nov 7, 2020
@lightdiscord lightdiscord marked this pull request as ready for review November 8, 2020 12:39
Copy link
Member

@kalbasit kalbasit left a comment

Choose a reason for hiding this comment

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

Also, have you tested this binary on Darwin?

@lightdiscord
Copy link
Member Author

Also, have you tested this binary on Darwin?

No, I wasn't able to test it on something else than x86_64-linux.

@ofborg ofborg bot requested a review from kalbasit November 9, 2020 11:35
@lightdiscord
Copy link
Member Author

It seems they've a tag for the 1.5.0 that I missed, I'll commit changes to package the 1.5.0 instead of the 1.4.5.

@lightdiscord lightdiscord marked this pull request as draft November 9, 2020 17:11
@kalbasit
Copy link
Member

kalbasit commented Nov 9, 2020

Also, have you tested this binary on Darwin?

No, I wasn't able to test it on something else than x86_64-linux.

No worries, I tested it for you:

$ nixpkgs-review pr 103037
[...]
$ ./results/protonmail-bridge/bin/protonmail-bridge --version
INFO[0000] Run app                                       appName="ProtonMail Bridge" args="[./results/protonmail-bridge/bin/protonmail-bridge --version]" build="2020-11-09T17:18:11+0000" pkg=cmd revision= runtime=darwin version=1.4.5-git
ProtonMail Bridge version 1.4.5-git () 2020-11-09T17:18:11+0000

@kalbasit
Copy link
Member

kalbasit commented Nov 9, 2020

I just noticed another problem. It seems that the binary is storing a timestamp to represent the build time. This is going to be problematic for the r13y effort. Can you please either set it to EPOCH 0 or set it to empty string? I'd pick whichever looks nicer in the output of --version.

@lightdiscord lightdiscord changed the title protonmail-bridge: 1.2.3-1 -> 1.4.5 protonmail-bridge: 1.2.3-1 -> 1.5.0 Nov 9, 2020
@lightdiscord lightdiscord marked this pull request as ready for review November 9, 2020 18:19
@ofborg ofborg bot requested a review from kalbasit November 9, 2020 18:30
@lightdiscord
Copy link
Member Author

Using the method described at r13y.com it seems that the build is now reproducible 🚀

@kalbasit
Copy link
Member

@GrahamcOfBorg build protonmail-bridge

@kalbasit kalbasit mentioned this pull request Nov 10, 2020
10 tasks
@afontaine
Copy link
Contributor

@GrahamcOfBorg build protonmail-bridge

@kalbasit is borg backed up or is this build just stuck?

@kalbasit
Copy link
Member

@afontaine I've been seeing issues with the Mac testing lately. Can you rebase over the latest master, I can test locally and merge.

@afontaine
Copy link
Contributor

@afontaine I've been seeing issues with the Mac testing lately. Can you rebase over the latest master, I can test locally and merge.

cc @lightdiscord

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Provide a minimal cli only version see #97991.

Reviewed-by: SuperSandro2000 <sandro.jaeckel@posteo.de>
Reviewed-by: kalbasit <github@kalbas.it>
@ofborg ofborg bot requested a review from kalbasit November 22, 2020 09:38
@prusnak prusnak linked an issue Nov 23, 2020 that may be closed by this pull request
@kalbasit
Copy link
Member

I'm away from my laptops until Thursday night. If someone can give this package a try on Mac, I'd appreciate it. Otherwise, I'll test and merge it then.

@afontaine
Copy link
Contributor

@kalbasit I found some time today to test it on my mac, builds and runs successfully 👍

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 103037 run on x86_64-linux 1

1 package built:
  • protonmail-bridge

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 103037 run on x86_64-darwin 1

1 package built:
  • protonmail-bridge

@SuperSandro2000 SuperSandro2000 merged commit 9c6a75e into NixOS:master Nov 26, 2020
@lightdiscord lightdiscord deleted the package-update/protonmail-bridge branch November 26, 2020 14:34
@kalbasit
Copy link
Member

Thank you @afontaine @SuperSandro2000 and happy Thanksgiving!

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.

Update protonmail-bridge to 1.3.x
5 participants