Summary: | SETUP : Database location under control panel doesn't match when using database-directory command line option [patch] | ||
---|---|---|---|
Product: | [Applications] digikam | Reporter: | Alphazo <kde-1091> |
Component: | Setup-Database | Assignee: | Digikam Developers <digikam-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | caulier.gilles, development, freisim93, phil.linttell, rharrison, swatilodha27 |
Priority: | NOR | ||
Version: | 5.1.0 | ||
Target Milestone: | --- | ||
Platform: | Arch Linux | ||
OS: | Linux | ||
Latest Commit: | https://commits.kde.org/digikam/e0f1d3dd4056deff260f673788af2cc6f9df1033 | Version Fixed In: | 5.6.0 |
Sentry Crash Report: | |||
Attachments: |
script to easily start multiple independent digikam instances
Add --config-file option and fix --database-directory Add --config-file option and fix --database-directory (still allow migration) Add --config option and fix --database-directory Add --config option and fix --database-directory |
Description
Alphazo
2009-12-11 17:00:27 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. 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. Dany, This file still valid using digiKam 2.x serie ? Gilles Caulier Dany, Is the file still valid using digiKam 5.1.0? Please test and provide feedback. 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. 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? 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. 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. 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. Created attachment 104573 [details]
Add --config-file option and fix --database-directory (still allow migration)
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. 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). 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? (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. Created attachment 104595 [details]
Add --config option and fix --database-directory
Created attachment 104597 [details]
Add --config option and fix --database-directory
Ok, I think I now understand you correctly and agree there is an issue with my suggestion. Cheers! *** Bug 267553 has been marked as a duplicate of this bug. *** Simon, I'm not sure if you take a care about bug #377587 with your current patch. Gilles Anyway, the patch sound like in the right way. Not yet tested. Gilles 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? If you have tested, it's fine for me... Gilles 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 |