Navigation Menu

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.2.6-1 #84430

Closed
wants to merge 4 commits into from
Closed

protonmail-bridge: 1.2.3-1 -> 1.2.6-1 #84430

wants to merge 4 commits into from

Conversation

CloudArtAdmins
Copy link
Contributor

Motivation for this change

Wanted more up to date version of package

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.

@ofborg ofborg bot requested a review from lightdiscord April 6, 2020 05:51
apeyroux
apeyroux suggested changes Apr 6, 2020
View reviewed changes
Copy link
Member

@apeyroux apeyroux left a comment •
edited

Choose a reason for hiding this comment

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

  1. the file ProtonMail_Bridge.desktop is no longer named ProtonMail_Bridge.desktop but protonmail-bridge.desktop
  2. this new version needs Qt_5.13 and for the moment it is not available in nixpkgs (Qt needs update to 5.14 and more maintainers #74355).
  3. we do not find the same sha256 for the src
@@ -2,7 +2,7 @@
 , libsecret, libGL, libpulseaudio, glib, wrapQtAppsHook, mkDerivation }:
 
 let
-  version = "1.2.3-1";
+  version = "1.2.6-1";
 
   description = ''
     An application that runs on your computer in the background and seamlessly encrypts
@@ -16,7 +16,7 @@ in mkDerivation {
 
   src = fetchurl {
     url = "https://protonmail.com/download/protonmail-bridge_${version}_amd64.deb";
-    sha256 = "032ggk9fvd19fbsqkzwzwh0hpyg8gpkrin71di7zsx6ias5innw1";
+    sha256 = "0iy2x9qnvhqrv6df0mjmxss9jddd64gh8wbadkn0cvrczcbwkz9l";
   };
 
   sourceRoot = ".";
@@ -55,7 +55,7 @@ in mkDerivation {
       --set-rpath "${rpath}" \
       $out/lib/protonmail-bridge
 
-    substituteInPlace $out/share/applications/ProtonMail_Bridge.desktop \
+    substituteInPlace $out/share/applications/protonmail-bridge.desktop \
       --replace "/usr/" "$out/" \
       --replace "Exec=protonmail-bridge" "Exec=$out/bin/protonmail-bridge"
   '';

apeyroux
apeyroux suggested changes Apr 6, 2020
View reviewed changes
Copy link
Member

@apeyroux apeyroux left a comment

Choose a reason for hiding this comment

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

s/protonmail_bridge/protonmail-bridge/

pkgs/applications/networking/protonmail-bridge/default.nix Outdated Show resolved Hide resolved
@CloudArtAdmins
Copy link
Contributor Author

Now awaiting an update for the qt package to update.
(#74355 )

@apeyroux
Copy link
Member

apeyroux commented Apr 7, 2020

We'll probably have to group the commit messages into one.

@CloudArtAdmins
Copy link
Contributor Author

CloudArtAdmins commented Apr 7, 2020 •
edited

Grouping the messages is fine by me.

@apeyroux
Copy link
Member

apeyroux commented Apr 7, 2020

I tested the build with this pr:#84232 and

modified   pkgs/top-level/all-packages.nix
@@ -21348,7 +21348,7 @@ in
 
   properties-cpp = callPackage ../development/libraries/properties-cpp { };
 
-  protonmail-bridge = libsForQt512.callPackage ../applications/networking/protonmail-bridge { };
+  protonmail-bridge = libsForQt514.callPackage ../applications/networking/protonmail-bridge { };
 
   protonvpn-cli = callPackage ../applications/networking/protonvpn-cli { };

1.2.6-1 build without errors and cli works but by using X there is this message:

./result/bin/protonmail-bridge: symbol lookup error: ./result/bin/protonmail-bridge: undefined symbol: _ZdlPvm, version Qt_5

Maybe a QT specialist has an idea? :)

@GRBurst
Copy link
Contributor

GRBurst commented Apr 19, 2020 •
edited

This error has been posted at stackoverflow. Maybe adding the mentioned QMAKE_CXXFLAGS += "-fno-sized-deallocation" flag helps :-)

@CloudArtAdmins
Copy link
Contributor Author

Protonmail has open-sourced the linux app for protonmail bridge.
So an alternative might be to create an app for cli only or maybe the code as clearer information on the issue.

@IvarWithoutBones
Copy link
Member

Why was this closed?

@CloudArtAdmins
Copy link
Contributor Author

A newer version was released and.. Really don't know if there is a more proper process for an outdated branch.

@talon
Copy link

talon commented Jul 20, 2020

@IvarWithoutBones I believe this is closed because 1) there's a newer version out but more importantly 2) ever since 1.2.4 the bridge depends on Qt 5.13 which means we have to wait for #84232 to be merged before protonmail-bridge can be updated to the latest version again

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gamb gamb gamb left review comments

@apeyroux apeyroux apeyroux requested changes

@lightdiscord lightdiscord Awaiting requested review from lightdiscord

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 10.rebuild-linux: 1-10 10.rebuild-linux: 1 12. first-time contribution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@CloudArtAdmins @apeyroux @GRBurst @IvarWithoutBones @talon @gamb @Janik-Haag