Bug 218297 - SETUP : Database location under control panel doesn't match when using database-directory command line option [patch]
Summary: SETUP : Database location under control panel doesn't match when using databa...
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Setup-Database (show other bugs)
Version: 5.1.0
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-11 17:00 UTC by Alphazo
Modified: 2017-03-18 15:01 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.6.0


Attachments
script to easily start multiple independent digikam instances (665 bytes, text/plain)
2017-03-14 16:30 UTC, Glenn Washburn
Details
Add --config-file option and fix --database-directory (8.22 KB, patch)
2017-03-14 23:29 UTC, Simon
Details
Add --config-file option and fix --database-directory (still allow migration) (9.72 KB, patch)
2017-03-14 23:52 UTC, Simon
Details
Add --config option and fix --database-directory (9.62 KB, patch)
2017-03-16 10:06 UTC, Simon
Details
Add --config option and fix --database-directory (9.61 KB, patch)
2017-03-16 10:15 UTC, Simon
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alphazo 2009-12-11 17:00:27 UTC
Version:           1.0 Release (using KDE 4.3.4)
OS:                Linux
Installed from:    Archlinux Packages

My database and pictures are stored on an external USB hard drive. In order to launch digiKam I use the following command: digikam --database-directory /media/PHOTOS/PHOTOS/Digikam

I noticed that in the Configuration panel, under 'Configure digiKam', the path mentioned for the database location doesn't match the one specified with the --database-directory. It shows the default "/home/dany/Pictures" which I don't use at all. It is rather confusing for the end user because at some point you are not exactly sure about where your database is stored.
Comment 1 Alphazo 2010-01-05 12:30:10 UTC
Things get worse when adding a new collection with the above configuration. 

Remember that the configuration page shows the default "/home/dany/Pictures" in the configuration panel rather than the specified command line parameter --database-directory /media/PHOTOS/PHOTOS/Digikam.

If I try to add a new collection like one on a removable drive then click OK at the bottom I would get the collection scan starting and then I get a pop-up windows saying (translated from French):

"You have selected « /home/dany/Pictures » as your new location for storing your database. A database already exists in this directory. Do you really wnat to use the existing file as database or delete it and copy it into the existing database?"

There are two buttons :
"Copy the current database" or "Use an existing file"

I clicked the little arrow on the top right as I don't want either of the proposed choices.
Comment 2 Russell Harrison 2010-08-14 18:45:51 UTC
I have also noticed this problem.  Partially I think because the --config command line options appears to have been removed.  I have several different collections of images I keep for very different purposes.  For example my images related to genealogy research.  The tag hierarchy that makes since for this repository is very different from my other photos.  I used to be able to just start digiKam from the command line with a different config and database-directory and everything worked great.  Now I have encountered this problem and when I switch back to my main repository the settings don't make since any more.
Comment 3 caulier.gilles 2011-12-15 08:56:43 UTC
Dany,

This file still valid using digiKam 2.x serie ?

Gilles Caulier
Comment 4 swatilodha27 2016-08-08 13:59:37 UTC
Dany,

Is the file still valid using digiKam 5.1.0?
Please test and provide feedback.
Comment 5 Alphazo 2016-08-12 14:05:48 UTC
Problem still exists with Digikam 5.1.0. 

I launch Digikam with digikam --database-directory /media/PHOTOS/PHOTOS/Digikam but the path of the database displayed under Settings/Configure Digikam/Database is not right since it shows the default one pointing to my home folder  /home/dany/Pictures.
Comment 6 Glenn Washburn 2017-03-13 21:27:40 UTC
How is this bug status still "UNCONFIRMED"?  I can confirm this still is a problem in 5.4 and I'm the third person to add confirmation.  This seems like a very simple thing to fix and is very annoying.  Is anyone working on this?
Comment 7 Glenn Washburn 2017-03-13 23:14:42 UTC
If the --data-directory argument is specified, then its value needs to be used as the argument to d->dbPathEdit->setFileDlgPath instead of "d->orgPrms.getCoreDatabaseNameOrDir()" in the function DatabaseSettingsWidget::setParametersFromSettings in file libs/database/utils/dbsettingswidget.cpp.  See: https://cgit.kde.org/digikam.git/tree/libs/database/utils/dbsettingswidget.cpp#n763

The value of the --data-directory argument is set to commandLineDBPath here: https://cgit.kde.org/digikam.git/tree/app/main/main.cpp#n146.  But it is only used to get the database paths initially and not saved.

I can think of several ways to get the command line argument value to where its needed, but I don't know the project well enough to feel comfortable making that choice.  For a dev on this project, this fix should be very easy and take minimal effort.
Comment 8 Glenn Washburn 2017-03-14 16:30:34 UTC
Created attachment 104564 [details]
script to easily start multiple independent digikam instances

Perhaps a better way to solve this is to allow different digikamrc files to be used, like addressed in bug 377587.

However, in the meantime, a work around for this issue is to use independent digikamrc files, which can be done by specifying a HOME envvar which uses a directory appropriately structured which has a digikamrc file unique to the database being used.  In Linux, the structure is that the the directory specified in HOME must have a ".config" subdirectory containing the digikamrc file.  It will be different for different platforms.  For clues to making this work on other platforms go to https://doc.qt.io/qt-5/qstandardpaths.html#StandardLocation-enum and look at the GenericConfigLocation path type for your platform.

Attached is a bash script that was tested with DK 5.4.0 to easily realize this.
Comment 9 Simon 2017-03-14 23:29:41 UTC
Created attachment 104572 [details]
Add --config-file option and fix --database-directory

The attached patch fixes this bug and https://bugs.kde.org/show_bug.cgi?id=377587.

In a first part the database path given by the command line option --database-directory is correctly displayed in configuration. However it does not make any sense to configure a command line option (i.e. runtime parameter), so the input fields are disabled.

The second part is to introduce a --config-file option, to specify a specific configuration file. This longer option (instead of just --config) was needed due to the weird behaviour of KSharedConfig with --config.
Comment 10 Simon 2017-03-14 23:52:01 UTC
Created attachment 104573 [details]
Add --config-file option and fix --database-directory (still allow migration)
Comment 11 Glenn Washburn 2017-03-15 06:52:56 UTC
Simon,

Why can't the --config option be used?  I know that KConfig::mainConfigName will check for the --config argument (see: ).  But why not use this existing functionality?  I think it should be as simple as adding an extra "parser.addOption" for --config, then not using the option and let KConfig do the rest.

Also, I'm not sure I agree with disabling the configuration path field.  This needlessly prevents the user from saving to another location.  By default it will be correct, so the user will have no need to change it unless she has a good reason to.

Otherwise, thanks for putting the work into making the patch.
Comment 12 Simon 2017-03-15 07:37:32 UTC
The problem with letting KSharedConfig handle the path is, that it is
not clearly specified what it does given an "invalid" path (e.g. a
directory, a non-existing parent directory), so I decided to handle this
myself and always use the standard location of the given path isn't valid.

What do you want to achieve by changing the database directory given at
command line in the configuration dialogue, which in general is
different from the one in the config file?
I mean even if that just resets the database directory at runtime and
leaves the config file be, that is in my opinion misleading, as on next
startup the new location won't be used (neither the config file nor the
command line option is automatically changed to the new path).
Comment 13 Glenn Washburn 2017-03-16 05:23:28 UTC
I would expect the configfile to have the "database Name*' properties changed if you change the database path in the configuration dialog.  Am I reading correctly that you're saying otherwise?  If not, then I'd agree that it would be misleading.  My point though, is that misleading or not, if the user does change this he probably has a good reason to.  Put another way, if you don't have a reason to change your database path, then you won't change that field and everything works fine.  Its comes down to a question of allowing more flexibility for unknown use cases at the expense of a user shooting themself in the foot, or to not allow this.  I'm in favor of less paternalism.  But personally, I'm not that adamant either way because even if that field is disabled, I can copy the database directory to a new location and have --database-directory point there for the same effect.

As for the cases where --config* is given a directory or the parent dir does not exist, I would suggest exiting with an error, instead of trying to recover.  If the user specified this option clearly they do not want the default config file, so why default to giving it to them?  If you agree with this logic, then I'd suggest using --config, exiting in either of those error conditions, and doing what I suggest initially otherwise.  What do you think?
Comment 14 Simon 2017-03-16 07:50:08 UTC
(In reply to Glenn Washburn from comment #13)
> I would expect the configfile to have the "database Name*' properties
> changed if you change the database path in the configuration dialog.  Am I
> reading correctly that you're saying otherwise?  If not, then I'd agree that
> it would be misleading.  My point though, is that misleading or not, if the
> user does change this he probably has a good reason to.  Put another way, if
> you don't have a reason to change your database path, then you won't change
> that field and everything works fine.  Its comes down to a question of
> allowing more flexibility for unknown use cases at the expense of a user
> shooting themself in the foot, or to not allow this.  I'm in favor of less
> paternalism.  But personally, I'm not that adamant either way because even
> if that field is disabled, I can copy the database directory to a new
> location and have --database-directory point there for the same effect.

You need to discern between two scenarios:
(1) Database location given with --database-directory.
(2) Database location taken from configuration file (i.e. now --database-directory command line option).
Obviously for (2) the path is still configurable and will be saved in configuration, otherwise what would be the point. However if you specify the path at runtime (1), which in general is a different path than in the config, you cannot modify the path in the configuration dialogue. Arguments as stated before: Either overwriting the configuration path, which is not visible in (1) or just changing the database location at runtime, which is not persistent after restart, is suboptimal. If a user specifically sets the database directory with a command line option, I don't see why he needed to change it in the GUI, just restart and use another command line option.

> As for the cases where --config* is given a directory or the parent dir does
> not exist, I would suggest exiting with an error, instead of trying to
> recover.  If the user specified this option clearly they do not want the
> default config file, so why default to giving it to them?  If you agree with
> this logic, then I'd suggest using --config, exiting in either of those
> error conditions, and doing what I suggest initially otherwise.  What do you
> think?

Sounds very sensibly actually.
Comment 15 Simon 2017-03-16 10:06:37 UTC
Created attachment 104595 [details]
Add --config option and fix --database-directory
Comment 16 Simon 2017-03-16 10:15:07 UTC
Created attachment 104597 [details]
Add --config option and fix --database-directory
Comment 17 Glenn Washburn 2017-03-16 19:01:37 UTC
Ok, I think I now understand you correctly and agree there is an issue with my suggestion.  Cheers!
Comment 18 caulier.gilles 2017-03-18 13:09:01 UTC
*** Bug 267553 has been marked as a duplicate of this bug. ***
Comment 19 caulier.gilles 2017-03-18 13:11:15 UTC
Simon,

I'm not sure if you take a care about bug #377587 with your current patch. 

Gilles
Comment 20 caulier.gilles 2017-03-18 13:12:26 UTC
Anyway, the patch sound like in the right way. Not yet tested.

Gilles
Comment 21 Simon 2017-03-18 13:16:25 UTC
I kind of do, but not in the way that is requested. I think it should be possible (and it is with my patch) to use a config location different than the database location. So to achieve what 377587 wants you have to specify both --config and --database-directory with the same path. It can definitely be closed when this is merged.

This is the second time I get a "patch looks good, but not tested". Does that mean I should commit if I have tested it (I did) or should I wait for your final approval?
Comment 22 caulier.gilles 2017-03-18 13:34:14 UTC
If you have tested, it's fine for me...

Gilles
Comment 23 Simon 2017-03-18 15:01:08 UTC
Git commit e0f1d3dd4056deff260f673788af2cc6f9df1033 by Simon Frei.
Committed on 18/03/2017 at 15:00.
Pushed by sfrei into branch 'master'.

Add --config option and fix --database-directory option

When database dir is specified at command line, the correct path is displayed
in the configuration widget, but the input field is disabled.
Related: bug 377587
FIXED-IN: 5.6.0

M  +2    -1    NEWS
M  +39   -4    app/main/main.cpp
M  +2    -2    libs/database/utils/dbmigrationdlg.cpp
M  +10   -1    libs/database/utils/dbsettingswidget.cpp
M  +2    -1    libs/database/utils/dbsettingswidget.h
M  +3    -0    libs/settings/applicationsettings.h
M  +10   -0    libs/settings/applicationsettings_miscs.cpp
M  +2    -0    libs/settings/applicationsettings_p.cpp
M  +1    -0    libs/settings/applicationsettings_p.h

https://commits.kde.org/digikam/e0f1d3dd4056deff260f673788af2cc6f9df1033