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

Jump to bottom

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 •
edited

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.

@lightdiscord lightdiscord mentioned this pull request Nov 6, 2020
Closed
10 tasks
@ofborg ofborg bot requested a review from kalbasit November 6, 2020 21:32
@lightdiscord lightdiscord force-pushed the package-update/protonmail-bridge branch from 82166a6 to 63d4f16 Compare November 8, 2020 12:36
@lightdiscord lightdiscord marked this pull request as ready for review November 8, 2020 12:39
kalbasit
kalbasit requested changes Nov 9, 2020
View reviewed changes
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?

pkgs/applications/networking/protonmail-bridge/default.nix Outdated Show resolved Hide resolved
@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.

@lightdiscord lightdiscord force-pushed the package-update/protonmail-bridge branch from 7ad4450 to 90a7f13 Compare November 9, 2020 11:21
@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 force-pushed the package-update/protonmail-bridge branch from da64167 to 5f38220 Compare November 9, 2020 18:18
@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
Closed
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

@lightdiscord
protonmail-bridge: 1.2.3-1 -> 1.5.0
472b6f4 
Provide a minimal cli only version see #97991.

Reviewed-by: SuperSandro2000 <sandro.jaeckel@posteo.de>
Reviewed-by: kalbasit <github@kalbas.it>
@lightdiscord lightdiscord force-pushed the package-update/protonmail-bridge branch from 5f38220 to 472b6f4 Compare November 22, 2020 09:27
@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
Update protonmail-bridge to 1.3.x #97991
Closed
@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
Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@prusnak prusnak prusnak approved these changes

@afontaine afontaine afontaine approved these changes

@kalbasit kalbasit Awaiting requested review from kalbasit

Assignees
No one assigned
Labels
8.has: package (new) 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update protonmail-bridge to 1.3.x
5 participants
@lightdiscord @kalbasit @afontaine @SuperSandro2000 @prusnak