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

nextcloud-client: switch to libsecret and use it by default #51044

Merged

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Nov 25, 2018

Motivation for this change

This change makes it easier for users to setup the nextcloud client with
GNOME keyring support as discussed in the IRC[1][2].

Additionaly we now use libsecret instead of libgnome-keyring which
integrates better in a GNOME setup (libgnome-keyring defaults to the
Gnome2 library)[3].

[1] https://logs.nix.samueldr.com/nixos/2018-11-24#1745871;
[2] https://logs.nix.samueldr.com/nixos/2018-11-24#1746033;
[3] #38266

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-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) (as stated in the commit message it has been discussed about this, we accept the increased closure size for usability)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

This change makes it easier for users to setup the nextcloud client with
GNOME keyring support as discussed in the IRC[1][2].

Additionaly we now use `libsecret` instead of `libgnome-keyring` which
integrates better in a GNOME setup (libgnome-keyring defaults to the
Gnome2 library)[3].

[1] https://logs.nix.samueldr.com/nixos/2018-11-24#1745871;
[2] https://logs.nix.samueldr.com/nixos/2018-11-24#1746033;
[3] NixOS#38266
@Ma27
Copy link
Member Author

Ma27 commented Nov 25, 2018

cc @jtojnar

@jtojnar
Copy link
Contributor

jtojnar commented Nov 25, 2018

LGTM.

cc @alexeymuranov can you test this?

@GrahamcOfBorg
Copy link

No attempt on x86_64-darwin (full log)

The following builds were skipped because they don't evaluate on x86_64-darwin: nextcloud-client

Partial log (click to expand)


a) For `nixos-rebuild` you can set
  { nixpkgs.config.allowUnsupportedSystem = true; }
in configuration.nix to override this.

b) For `nix-env`, `nix-build`, `nix-shell` or any other Nix command you can add
  { allowUnsupportedSystem = true; }
to ~/.config/nixpkgs/config.nix.


@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: nextcloud-client

Partial log (click to expand)

shrinking /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/lib64/nextcloud/libocsync.so.2.5.0
shrinking /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/bin/.nextcloud-wrapped
shrinking /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/bin/nextcloudcmd
strip is /nix/store/rpbg8gmqxhz8g61p1plz5d2srs84pvmv-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/lib64  /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/bin
patching script interpreter paths in /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0
checking for references to /build in /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0...
moving /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/lib64/* to /nix/store/1acxgwj0jiyi0b6wwygx5akl5y2m68bi-nextcloud-client-2.5.0/lib
postPatchMkspecs
postPatchMkspecs

@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: nextcloud-client

Partial log (click to expand)

make[3]: *** [Makefile.gn_run:336: run_ninja] Error 1
make[3]: Leaving directory '/build/qtwebengine-everywhere-src-5.11.1/src/core'
make[2]: *** [Makefile:80: sub-gn_run-pro-make_first] Error 2
make[2]: Leaving directory '/build/qtwebengine-everywhere-src-5.11.1/src/core'
make[1]: *** [Makefile:79: sub-core-make_first] Error 2
make[1]: Leaving directory '/build/qtwebengine-everywhere-src-5.11.1/src'
make: *** [Makefile:47: sub-src-make_first] Error 2
builder for '/nix/store/280x803mp96p5yyrg5lv83644g62v2ya-qtwebengine-5.11.1.drv' failed with exit code 2
cannot build derivation '/nix/store/zwa54whk6hlnqgwngdn4fbpfbmjp43n8-nextcloud-client-2.5.0.drv': 2 dependencies couldn't be built
error: build of '/nix/store/zwa54whk6hlnqgwngdn4fbpfbmjp43n8-nextcloud-client-2.5.0.drv' failed

@M-Gregoire
Copy link
Member

M-Gregoire commented Dec 1, 2018

First of all, thank you for taking the time to work on the nextcloud client!

The issues

I've tried using you branch for nextcloud-client and made a couple tests:

  • Each time I restart nextcloud-client, I'm prompted with You have been logged out of my@email.com as user me. Please login again, with the following screen.
    2018-12-01-205551_810x664_scrot

I have to enter my username/password then, it works.

  • I also tried with an own cloud server (Not sure the compatibility with owncloud is very important...) and it work only right after setup. After the first restart, it asks for the username/password but redirect to the home page of owncloud wepapp instead of authenticating.
    2018-12-01-191956_812x663_scrot

The install

Just nextcloud-client installed from your branch without any override.

I still have gnome-keyring enabled but I don't believe this should be a problem:

  services.gnome3 = {
      gnome-keyring.enable = true;
      seahorse.enable = true;
  };

Is there anything else that should be installed ?

Note: The prompt is not the same as what I had when the keyring was improperly configured in the old version of nextcloud-client, I just had to type my password again, while in this case a get an iframe to get the token.

EDIT: I tried using an App token instead of my username/password but same result.

@alexeymuranov
Copy link
Contributor

alexeymuranov commented Dec 1, 2018

I havn't tried to test it yet, but here are a few observations about nextcloud-client version 2.5.0. It seems to be quite different from version 2.3.3 and incompatible with it: upgrading-downgrading-upgrading seems to cause losing connection configuration. There are a number of issue reported about version 2.5.0, and on Ubuntu i experienced file disappearance with it.

@Ma27
Copy link
Member Author

Ma27 commented Dec 2, 2018

@M-Gregoire thanks a lot for your detailed feedback! I'll look into it tomorrow.
@worldofpeace as you 👍-ed @M-Gregoire's comment, did you have similar results?

@alexeymuranov I doubt that they actively support downgrades. Before I filed #50463 I tested this on my local Nextcloud setup and it managed to migrate my configurations and it found an existing sync in ~/Nextcloud.
Regarding the issue with symlinks: the issue you've linked (nextcloud/desktop#250) states that symlinks in folder syncs aren't support officially right? But thanks a lot for the research, I'm a bit tired ATM, so I'll have a look at all the issues tomorrow :)

@alexeymuranov
Copy link
Contributor

@Ma27, they are free to support or not support symlinks, but when files disappear, it is a bug IMO, and if symlinks are not supported, no changes to directory contents should be made, and otherwise synchronisation should be done properly.

@worldofpeace
Copy link
Contributor

worldofpeace commented Dec 3, 2018

@worldofpeace as you +1-ed @M-Gregoire's comment, did you have similar results?

@Ma27 Upon testing, within gnome3, I did notice the first issue @M-Gregoire pointed out.
Looks like it didn't unlock the keyring.

I've also tested kwallet and it operates correctly.

@Ma27
Copy link
Member Author

Ma27 commented Dec 4, 2018

Each time I restart nextcloud-client, I'm prompted with You have been logged out of my@email.com as user me. Please login again, with the following screen.

hmm... I use my change already in my local setup and it works fine. Simply opening nextcloud and I'm logged in and it tries to sync changes.

Not sure the compatibility with owncloud is very important

Not sure either. But please not that there's pkgs.owncloud-client as well. I can't test this though, I'm using Nextcloud only at work and privately.

Is there anything else that should be installed ?

I don't think so. I figured that if the daemon is running, this should be fine.

Note: The prompt is not the same as what I had when the keyring was improperly configured in the old version of nextcloud-client, I just had to type my password again, while in this case a get an iframe to get the token.

Which prompt do you mean? When I login in a fresh setup, the keyring asks me to unlock it with my master passphrase and then I'm logged in.

Btw the more I think about this: is it possible that you had 2.3 before testing this? I'm not sure, but this may be related to the config "migration" that happens when using the 2.5 client with 2.3 config.

Upon testing, within gnome3, I did notice the first issue @M-Gregoire pointed out.
Looks like it didn't unlock the keyring.

Thanks for trying this out. Maybe this is gnome related (as said, I couldn't reproduce this yet), I'll try this out in a Gnome VM later this week ;)

@M-Gregoire
Copy link
Member

@Ma27 Thank you for your extensive answer! I will try to investigate further.

Note that I'm not using Gnome but i3, I only use gnome3 services to enable the keyring

  # Seahorse depends on dconf
  programs.dconf.enable = true;
  # gconf needed by thunderbird
  # See https://github.com/NixOS/nixpkgs/issues/13287
  services.dbus.packages = [ pkgs.gnome3.dconf pkgs.gnome2.GConf ];
  services.gnome3 = {
      gnome-keyring.enable = true;
      seahorse.enable = true;
  };
  security.pam.services.lightdm.enableGnomeKeyring = true;

I'm unsure this is the best way to do it however. I've also noticed though seahorse that my keyring doesn't seems to unlock at startup so that might not by a nextcloud problem, even though It used to work fine.

I'll keep you updated.

@worldofpeace
Copy link
Contributor

Maybe this is gnome related (as said, I couldn't reproduce this yet), I'll try this out in a Gnome VM later this week ;)

The actual testing I did was within a gnome vm, forgot to mention that.

@Ma27
Copy link
Member Author

Ma27 commented Dec 6, 2018

I'm unsure this is the best way to do it however. I've also noticed though seahorse that my keyring doesn't seems to unlock at startup so that might not by a nextcloud problem, even though It used to work fine.

I don't know enough about the keyring to help you here. Is this a known issue? Otherwise I'd recommend to you ask in #nixos (or similar), maybe there are people who successfully use Gnome keyring on a non-gnome setup :)

@M-Gregoire
Copy link
Member

I've migrated to kwallet and everything worked perfectly on the first try so I'll probably stay with that!

@Ma27
Copy link
Member Author

Ma27 commented Dec 16, 2018

nixpkgs now uses nextcloud-client v2.5.1 since #52354, does this fix the issues for some of you here?

@jtojnar jtojnar merged commit f10f54b into NixOS:master Dec 17, 2018
@Ma27 Ma27 deleted the use-libsecret-by-default-in-nextcloud-client branch December 17, 2018 23:48
@M-Gregoire
Copy link
Member

@Ma27 Thank you for updating nextcloud-client. However, since v.2.5.1 got merged, the integration with kwalletseems to be broken as nextcloud-client ask for my password every time again.

@Ma27
Copy link
Member Author

Ma27 commented Dec 18, 2018

cc @flokli do you have an idea how this could be related to nextcloud-client?

@flokli
Copy link
Contributor

flokli commented Dec 18, 2018

@M-Gregoire, @Ma27 I might not be the most qualified person, given I don't run a kde environment, but i3 with desktopManager.gnome.enable = true.

All I can tell is that every time I start nextcloud-client, a kwalletd5 is spawned, too, and I don't get asked for a password.

@M-Gregoire
Copy link
Member

@Ma27 @flokli This isn't a problem with nextcloud but with my wallet which didn't unlocked when my session started. I'll investigate to see if this is a nix problem or whether I did something wrong in my config.

I'm sorry for the false alarm!

@caugner
Copy link
Contributor

caugner commented Jan 22, 2019

@M-Gregoire I'm getting the same You have been logged out ... prompts and nextcloud-client crashes as soon as I login (see nextcloud/desktop#1007 (comment)). Were you able to make it work?

@Ma27 Is there any chance to override libsecret with libgnome-keyring now? nextcloud-client 2.5.1 has been working on the 18.09 channel (a21dad5), but either this change (f10f54b) or the one after (ead6899) must have broken it.

@Ma27
Copy link
Member Author

Ma27 commented Jan 22, 2019

On 18.09 you need to override nextcloud-client-unwrapped. It seems as we didn't backport the latest commit which removed the wrapper (again).

I'll check tomorrow if there's a need to backport the most recent patches ok? :)

@caugner
Copy link
Contributor

caugner commented Jan 22, 2019

@Ma27 Sorry, to clarify: The latest build on 18.09 seems (i.e. a21dad5) to work, but when I switched to unstable (i.e. ead6899) it didn't work any more.

@Ma27
Copy link
Member Author

Ma27 commented Jan 22, 2019

Ah sorry, missed that. I don't use unstable, so I didn't notice, but nextcloud-client indeed breaks on startup on unstable. I have a theory what's causing this, I'll try to fix it :)

Ma27 added a commit to Ma27/nixpkgs that referenced this pull request Jan 22, 2019
The QT_PLUGIN_PATH couldn't find the needed xcb plugin.

See also NixOS#51044
@Ma27
Copy link
Member Author

Ma27 commented Jan 22, 2019

can you please have a look at #54484, I can run nextcloud-client from master now.

Ma27 added a commit that referenced this pull request Jan 27, 2019
The QT_PLUGIN_PATH couldn't find the needed xcb plugin.

See also #51044

(cherry picked from commit b525493)
danbst pushed a commit to danbst/nixpkgs that referenced this pull request Jan 31, 2019
The QT_PLUGIN_PATH couldn't find the needed xcb plugin.

See also NixOS#51044
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.

None yet

8 participants