Don't create 'data' subdirectory on Linux

[lbilli]

Any reason to use on Linux the unusual path

~/.local/share/data/qBittorrent

instead of

~/.local/share/qBittorrent

?

[glassez]

At least you break existing installations, isn't it?

[FranciscoPombal]

Upgrade path could be as follows (semi pseudo-code):

IF EXISTS ~/.local/share/data/qBittorrent THEN:
    mkdir -p ~/.local/share/qBittorrent
    mv -t ~/.local/share/qBittorrent/ ~/.local/share/data/qBittorrent/*

• Pro: better compliance with the XDG Base Directory Specification, since on Ubuntu and most distros $XDG_DATA_HOME is ~/.local/share
• Con: downgrades from the version that implements this would require manual intervention from the user, to copy everything in ~/.local/share/qBittorrent/ back to ~/.local/share/data/qBittorrent/

[FranciscoPombal]

On second thought, are you sure the code you changed is what makes qBittorrent create the directory in the first place, or just a function called to return what the path actually is, after having been created elsewhere? Especially this comment seems to imply the second option:

qBittorrent/src/base/private/profile_p.cpp

Line 79 in d09c5e5

// on Linux gods know why qBittorrent creates 'data' subdirectory in ~/.local/share/

EDIT: not a problem, see next post

[Kolcha]

I agreed, data folder looks strange, very unusual and even annoying, but I thought it was left for history reasons (compatibility)... but the comment that @FranciscoPombal found made me think "what's really going on?"
I'll investigate it these days and publish results here, this is not a problem for me even if I'll have to dig into Qt' sources "in the search of truth". Fortunately, I have suitable Linux environment for that.

[lbilli]

FYI
I'm running a patched version of qBittorrent on my machine and I don't see any problems.

[Kolcha]

Confirmed, modified function returns path which is used everywhere, i.e. first time when folder must be created. data part comes exactly from this function.

[FranciscoPombal]

Ok, upon a closer look, I can confirm that this patch should work as intended. That place is where, in fact, the directory is set to be created as such.

For anyone interested, here is the full code path:

0. The application creates an instance of Profile (Profile appears to be a singleton class).
   

   qBittorrent/src/app/application.cpp

   Line 152 in d09c5e5

   Profile::initialize(profileDir, m_commandLineArgs.configurationName,

1. The Profile constructor calls ensureDirectoryExists with the SpecialDiretory::Data parameter
   

   qBittorrent/src/base/profile.cpp

   Line 42 in d09c5e5

   ensureDirectoryExists(SpecialFolder::Data);

2. Inside ensureDirectoryExists the following lines sets the path to be created:
   

   qBittorrent/src/base/profile.cpp

   Line 101 in d09c5e5

   const QString locationPath = location(folder);

3. Inside the location function this gets called, which is the function that is changed in this patch:
   

   qBittorrent/src/base/profile.cpp

   Line 77 in d09c5e5

   result = m_profileImpl->dataLocation();

4. Back in ensureDirectoryExists, the directories actually get created with the location specified previously, and qBitorrent goes about the rest of its business.
   

   qBittorrent/src/base/profile.cpp

   Line 102 in d09c5e5

   if (!locationPath.isEmpty() && !QDir().mkpath(locationPath))

I would still recommend providing an upgrade path in this PR, as outlined above.

[j1warren]

I think the right way is to check if 'data' folder exists, then continue using it.
If it doesn't, generate and use the new path without it.

That way no old installations brake, but newer ones and those where people moved it by hand will use cleaner path.
And then somewhere at version 6.0 brake when using 'data' path.

[FranciscoPombal]

@lbilli If you want this to get merged, you still need to provide an upgrade path, either
#11644 (comment) or #11644 (comment)

[lbilli]

@FranciscoPombal
I added the logic to use /data/ if it already exists, as suggested above.
Feel free to modify or improve if necessary.

[NotTsunami]

@lbilli Can you rebase and implement the suggested changes? The file has moved to src/base/profile_p.cpp. It would be nice to see this make it in the upcoming release.

[lbilli]

Rebased and added a warning.
However, I'm not sure what's the best practice to log a warning at this stage when the Logger is not active yet.

[NotTsunami]

@Chocobo1 @glassez I know you hate these pings, but I think this is a good candidate to get in 4.3.x. Can you guys review this please?

[glassez]

Please squash your commits into one.

[FranciscoPombal]

Rebased and added a warning.
However, I'm not sure what's the best practice to log a warning at this stage when the Logger is not active yet.

Oh, yes. I guess the warning is just fine then.

[Chocobo1]

@lbilli
Thank you!