nextcloud / desktop

💻 Desktop sync client for Nextcloud
https://nextcloud.com/install/#install-clients
GNU General Public License v2.0
2.97k stars 784 forks source link

[Bug]: Invalid warning: `Failed to move the old config directory to its new location` #6201

Open robeke opened 10 months ago

robeke commented 10 months ago

⚠️ Before submitting, please verify the following: ⚠️

Bug description

During startup, the following warning is logged: nextcloud[2402]: nextcloud.gui.application: Failed to move the old config directory to its new location ( "/home/xxxxxxx/.local/share/Nextcloud" to "/home/xxxxxxx/.config/Nextcloud" ) I believe this is an invalid warning since my ~/.config/Nextcloud directory already contains nextcloud.cfg and a series of monthly backups. The directory ~/.local/share/Nextcloud does not contain nextcloud.cfg but does contain a *_sync.log file for each folder sync connection. There is also a *_sync.log file for each folder sync connection under ~/.config/Nextcloud but these no longer appear to be up-to-date compared to those under ~/.local/share/Nextcloud. Is it possible that the *_sync.log files in ~/.config/Nextcloud are OBE and need to be removed? The Desktop Client appears to work at this point but I would have no way to know if the cause of this warning would eventually lead to a break in client functionality.

Steps to reproduce

Start the Nextcloud Client and view the message in the system log.

Expected behavior

The message should either correctly state the condition it is warning about or not be generated.

Which files are affected by this bug

nextcloud.cfg

Operating system

Linux

Which version of the operating system you are running.

Manjaro

Package

Distro package manager

Nextcloud Server version

24.0.12

Nextcloud Desktop Client version

3.10.1

Is this bug present after an update or on a fresh install?

Updated from a minor version (ex. 3.4.2 to 3.4.4)

Are you using the Nextcloud Server Encryption module?

Encryption is Enabled

Are you using an external user-backend?

Nextcloud Server logs

No response

Additional info

No response

robeke commented 10 months ago

I also observed one warning for each folder sync connection log immediately following the one above:

Nov 06 14:13:58 dv6-2150 nextcloud[2402]: nextcloud.gui.application: Fallback move of  "xxxxxx_sync.log" also failed
Nov 06 14:13:58 dv6-2150 nextcloud[2402]: nextcloud.gui.application: Fallback move of  "xxxx_sync.log" also failed
Nov 06 14:13:58 dv6-2150 nextcloud[2402]: nextcloud.gui.application: Fallback move of  "xxxxx_sync.log" also failed
Janov911 commented 10 months ago

same here, on my steam deck (checking other machines this evening):

nextcloud.gui.application: Migrating old config from "/home/deck/.var/app/com.nextcloud.desktopclient.nextcloud/data/Nextcloud" to "/home/deck/.var/app/com.nextcloud.desktopclient.nextcloud/config/Nextcloud" nextcloud.gui.application: Failed to move the old config directory to its new location ( "/home/deck/.var/app/com.nextcloud.desktopclient.nextcloud/data/Nextcloud" to "/home/deck/.var/app/com.nextcloud.desktopclient.nextcloud/config/Nextcloud" ) nextcloud.gui.application: Will move the individual files ("ccc.log", "ccc.log", "cccc.log", "cc-cccc.log", "cccc.log", "ccc.log", "ccc.log", "ccc.log", "ccc.log") nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc-ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed nextcloud.gui.application: Fallback move of "ccc.log" also failed

note: ccc = file name

joshtrichards commented 10 months ago

The migration only runs if oldDir exists:

https://github.com/nextcloud/desktop/blob/ab3e4e724f18601e61dca0634f31ef7571f9b3f6/src/gui/application.cpp#L492-L493

The migration itself normally would rename oldDir to newDir (aka: confDir) and if that fails logs the message you're seeing:

https://github.com/nextcloud/desktop/blob/ab3e4e724f18601e61dca0634f31ef7571f9b3f6/src/gui/application.cpp#L504-L505

Then it falls back on trying to move the files within oldDir one-by-one:

https://github.com/nextcloud/desktop/blob/ab3e4e724f18601e61dca0634f31ef7571f9b3f6/src/gui/application.cpp#L507-L517

There are some other spots where migration can happen - e.g.:

https://github.com/nextcloud/desktop/blob/ab3e4e724f18601e61dca0634f31ef7571f9b3f6/src/libsync/configfile.cpp#L343-L366

Notably, the _sync.log files figure out their path here:

https://github.com/nextcloud/desktop/blob/ab3e4e724f18601e61dca0634f31ef7571f9b3f6/src/gui/syncrunfilelog.cpp#L35

During startup, the following warning is logged: nextcloud[2402]: nextcloud.gui.application: Failed to move the old config directory to its new location ( "/home/xxxxxxx/.local/share/Nextcloud" to "/home/xxxxxxx/.config/Nextcloud" ) I believe this is an invalid warning since my ~/.config/Nextcloud directory already contains nextcloud.cfg and a series of monthly backups.

It's valid because for some reason the simple rename migration didn't originally cover you for some reason. It just means the fallback mode kicks in (one-by-one above). The fallback kicking in now makes sense because you have both old and new directories now so a simple rename of the old directory won't work.

The questions are:

The directory ~/.local/share/Nextcloud does not contain nextcloud.cfg but does contain a _sync.log file for each folder sync connection. There is also a _sync.log file for each folder sync connection under ~/.config/Nextcloud but these no longer appear to be up-to-date compared to those under ~/.local/share/Nextcloud. Is it possible that the *_sync.log files in ~/.config/Nextcloud are OBE and need to be removed? The Desktop Client appears to work at this point but I would have no way to know if the cause of this warning would eventually lead to a break in client functionality.

P.S. Manjaro's package data is weird. It says 3.10.1 on the listing on their web site but when you click on it it's for 3.9 and links to an invalid broken build source link.

robeke commented 10 months ago

@joshtrichards Thanks for helpfull insight. On my Manjaro desktop, the _sync.log files under .local/share/Nextcloud have a current modification date (Nov 16) whereas those under .config/Nextcloud have not been modified since Nov 6. The update history for my system shows 23-11-06T07:50:38-0500] [ALPM] upgraded nextcloud-client (2:3.10.0-2 -> 2:3.10.1-1) so it appears that after that upgrade, the _sync.log files under .config/Nextcloud are being ignored.

Based on the code blocks you included, it makes sense that the _sync.log files are expected to be found in the standard data location whereas the nextcloud.cfg is expected to be in the standard config location. Given this, "oldDir" for the _sync.log files would be .config/Nextcloud directory. However, this was created by Nextcloud more than two years ago and as the standard config location is still being used to contain the nextcloud.cfg file (and backups) as well as cookies0.db, sync-exclude.lst, and a logs subdirectory containing the application log, i.e., 20231116_0812_nextcloud.log.1 with previous logs as archives. In this configuration, renaming oldDir to newDir does not really make sense. It may just be simply that the *_sync.log files under .config/Nextcloud are now OBE and should be removed.

mephinet commented 10 months ago

The old directory is re-created every time:

rm -rf ~/.local/share/Nextcloud/
nextcloud &
ls -d ~/.local/share/Nextcloud/
/home/.../.local/share/Nextcloud/

and it contains a new log file for each sync target. I'm here on Linux with version 3.10.1

SlappyHours commented 9 months ago

Same problem here on Linux with 3.10.1: .local/share/Nextcloud gets recreated. Next start shows error message because of failed migration. The only way to prevent the error message is to delete .local/share/Nextcloud manually before each start of Nextcloud Client.

adrianvg commented 9 months ago

Just updated the desktop client Linux Appimage from https://nextcloud.com/install/#install-clients from version 3.10.0 to the latest 3.11.0 and seeing the same thing.

nextcloud.gui.application: Migrating old config from "/home/username/.local/share/Nextcloud" to "/home/username/.config/Nextcloud"
nextcloud.gui.application: Failed to move the old config directory to its new location ( "/home/username/.local/share/Nextcloud" to "/home/username/.config/Nextcloud" )
nextcloud.gui.application: Will move the individual files ("Nextcloud_sync.log")
nextcloud.gui.application: Fallback move of  "Nextcloud_sync.log" also failed

Tried deleteing /home/username/.local/share/Nextcloud but still getting the error message. The client still seems to work though.

Is this is a cosmetical problem only, or may things keel over later?

djtulan commented 7 months ago

Also on 3.11.1 Appimage Client

ecsgh commented 7 months ago

I can confirm this. If you delete .local/share/Nextcloud and start nextcloud, no error message appears. But only on the first start. After that, however, the error "Failed to move the old config directory to its new location" occurs again and again All files are migrated to ./config/Nextcloud except for the log file Nextcloud_sync.log. This is always created under .local/share/Nextcloud.

Tachi107 commented 6 months ago

@joshtrichards said:

why are your _sync.log logs being written to your old path? 🤔

That's because, as you mentioned, the _sync.log files created by the SyncRunFileLog::start() function are created in the QStandardPaths::writableLocation(QStandardPaths::AppDataLocation) directory, which expands to ~/.local/share/Nextcloud/.

Then, on the following application launch, the Application::setupConfigFile() function sees the previously created ~/.local/share/Nextcloud/ directory, and (wrongly) tries to move it to ~/.config/Nextcloud/, failing because it already exists.

In my opinion, to properly fix this issue a couple of things have to change:

  1. Logs shouldn't be written in ~/.local/share/Nextcloud/ nor in ~/.config/Nextcloud/logs/, but should be instead be created in $XDG_STATE_HOME (~/.local/state/), as explicitly mentioned in the XDG Base Directory Specification.
  2. The migration logic should be modified to only try to move .cfg files from ~/.local/share/Nextcloud/ to /.config/Nextcloud/, and not the whole directory. Then, it should remove all files from ~/.local/share/Nextcloud/ matching known patterns like _sync.log, and try to delete the ~/.local/share/Nextcloud/ folder as if using std::filesystem::remove() so that the directory doesn't get deleted if users put some custom file there.

As far as I understand, this should fix this issue, and make the desktop client a bit more conforming to the XDG guidelines :)

Edit: QStandardPaths doesn't seem to have an enum value representing $XDG_STATE_HOME, but I guess QStandardPaths::CacheLocation (expanding to ~/.local/cache/) should be a good enough alternative.

johanneskastl commented 4 months ago

Any news on this? (I am not sure if this issue is the cause, but I have 700MB of logs in .config/Nextcloud currently. Not sure if that is caused by the multiple attempts to migrate things or if it is unrelated)

djtulan commented 3 months ago

It seems it works if you delete the folder ~/.local/share/Nextcloud/ . No new folder is created which caused the bug.

mjg commented 3 months ago

At least over here with Nextcloud version 3.12.5git Git revision 29df09454e6da423d4f3c788a491b1eb8636236d, the folder ~/.local/share/Nextcloud/ is recreated after deleting it (of course I stopped the client, deleted the folder, ...). This is on Fedora 40.

djtulan commented 3 months ago

I use Nextcloud-3.13.0-x86_64.AppImage and recently I upgraded my Kubuntu from 22.04 to 24.04 LTS (maybe that has an impact?)

ecsgh commented 3 months ago

It seems it works if you delete the folder ~/.local/share/Nextcloud/ . No new folder is created which caused the bug.

This is not right. Folder will recreated. See my comment on 14 February.

djtulan commented 3 months ago

Oh sorry, I was wrong. I paused synchronization, that's why it seemed to work. I started a few times. Still not fixed, sorry :(

leogzyl commented 2 months ago

Edit: QStandardPaths doesn't seem to have an enum value representing $XDG_STATE_HOME, but I guess QStandardPaths::CacheLocation (expanding to ~/.local/cache/) should be a good enough alternative.

@Tachi107 Don't know if I'm missing something, but how about StateLocation? https://doc.qt.io/qt-6/qstandardpaths.html

Edit: I guess that path is new in Qt 6.7 and Nextcloud client uses a previous version? :(

Tachi107 commented 2 months ago

Yeah, that's the direct Qt abstraction. It has been added in Qt 6.7 though, and wasn't available when I submitted my suggestion.

tiesel commented 1 month ago

I had the problem that nextcloud client was hangig for long times and sometimes even wasn't able to sync at all. This could be worked around here under Fedora 40 with client 3.13.2-1 and now the client is responsive again and syncs as it should:

This creates even more journalctl entries "Failed to move" and "Fallback move ... also failed" but in the end it works. We shouldn't forget to check from time to time if the problem is fixed and to take away the symlink then.

erebion commented 6 days ago

If there are newer (by date and time) bla_sync.log files in the old ~/.local/share/Nextcloud/ than in the new ~/.config/Nextcloud folder: Copy them from old to new

Hell no, it should NEVER place log files in the config directory!

Please, just read the spec everyone: https://specifications.freedesktop.org/basedir-spec/latest/index.html

It clearly states the following:

$XDG_CONFIG_HOME defines the base directory relative to which user-specific configuration files should be stored. If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used.

And for the state directory:

The $XDG_STATE_HOME contains state data that should persist between (application) restarts, but that is not important or portable enough to the user that it should be stored in $XDG_DATA_HOME. It may contain:

actions history (logs, history, recently used files, …)

current state of the application that can be reused on a restart (view, layout, open files, undo history, …)

Please... Just don't ever put log files in a config directory.

That's like putting the Nextcloud server log in /etc/nginx/nextcloud.log, who the hell would do that?!

There's a standard for a reson.

One of those reasons is that users could sync their config directories between machines if they only contained config... And not the state. But this just breaks every time an application puts stuff there that is definitely not config. Electron, for instance, just puts the bloody cache in that damn directory. WTF, cache is not config.

Another one is that users could easily backup their config. If users backup their Nextcloud config, they will also accidentally backup log files, which just unnecessarily use up disk space, even though it has never been intended to backup those as well.

Just follow the standard please. Some very clever people put a lot of thought into it.

Tachi107 commented 6 days ago

Hi @erebion, we clearly all know that the Nextcloud client is currently doing stuff wrong, and in https://github.com/nextcloud/desktop/issues/6201#issuecomment-1962299217 I've already mentioned what should be done instead. Please calm down :)

erebion commented 6 days ago

I couldn't not spot any link to the spec yet, nor an excerpt. It seemeed only logical to clarify.