Bug 334393

Summary: Import from a .kim-file fails because path for image files is incorrect
Product: [Applications] kphotoalbum Reporter: Andreas Schleth <schleth_es>
Component: Import/ExportAssignee: KPhotoAlbum Bugs <kpabugs>
Severity: normal CC: johannes, tmp33
Priority: NOR    
Version: 4.4   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In:
Attachments: screenshot of kpa-error message and file manager window
screenshot: folders in kpa after import

Description Andreas Schleth 2014-05-05 21:55:28 UTC
Importing images from another directory (via .kim-file) fails because path names are resolved wrong - see details below.

Reproducible: Always

Steps to Reproduce:
1. Generate an export file with images copied next to the .kim file in some directory.
2. Inside the .kim file the images do not have a path whatsoever, only "DSC5384.jpg" or so.
3. The .kim file lies at (e.g.) /local_fast/as/transfer
4. The database to import into lies at (e.g.) /home/usr/share/fotos (with a folder structure underneath)
Actual Results:  
Kphotoalbum complains, that it cannot copy from either "/home/usr/share/fotos/local_fast/as/transfer/DSC5384.jpg" or from "/home/usr/share/fotos/DSC5384.jpg"
Both path names are wrong.  The path should be constructed relative to the location of the .kim file the images are imported from AND NOT relative to the database the images are to be transferred into.

(Then KPA crashes after hitting "cancel" - but this is NOT the issue here!)

Expected Results:  
Image files should be copied from (e.g.) /local_fast/as/transfer into the destination folder.
Path names for image source on copy should be relative to the .kim-file (as was the behaviour in previous releases (4.1.1)).
Comment 1 Andreas Schleth 2014-05-05 21:58:03 UTC
Created attachment 86478 [details]
screenshot of kpa-error message and file manager window

The screenshot shows what goes wrong.  The file manager displays the actual path name and in the error message the two failed attempts are shown.
Comment 2 Andreas Schleth 2014-05-05 22:47:43 UTC
To add insult to injury: 
Basically the same bug is uncovered if importing from an integral .kim-file (with images inside):
1.) export images to .kim-file and copy images into this file - result one BIG .kim-file
2.) import this kim file (and create a new folder for the imported images in KPA):
3.) import dialog finishes without a hitch, BUT:
   a) all images are shown as blank ("file not available")
   b) the newly created folder is nowhere to be found (in KPA) - on disk things are as they should be
   c) inside the index.xml all new images are imported WITHOUT path:
         <image file="DSC_5260.jpg" label ...
This should be:
        <image file="my_existing_path/newly_created_folder_name/DSC_5260.jpg" label ...

This, the import feature has been completely destroyed.
Comment 3 Andreas Schleth 2014-05-05 22:48:15 UTC
*Thus* ... (not "This")
Comment 4 Johannes Zarl-Zierl 2014-05-10 20:22:59 UTC
Hi Andreas,

I'm currently looking into this. The situation seems partially fixed in git master:

Importing a .kim file with inline images *does* work for me in git master, but the folder information is thrown away. I.e. images are added to the main image folder. So there is definitely something that needs fixing :-|
Comment 5 Johannes Zarl-Zierl 2014-05-10 21:16:39 UTC
Git commit b70289173b9f3ef204c624bee46670842c79d5cc by Johannes Zarl.
Committed on 10/05/2014 at 21:11.
Pushed by johanneszarl into branch 'master'.

Fix image import for kim files with external images.

Use relative paths in ImportHandler::copyNextFromExternal().

M  +1    -1    ImportExport/ImportHandler.cpp

Comment 6 Andreas Schleth 2014-05-11 16:02:06 UTC
Created attachment 86584 [details]
screenshot: folders in kpa after import

Test with new build from github (11 may ca. 15:00 MESZ)
Comment 7 Andreas Schleth 2014-05-11 16:15:16 UTC
Hi Johannes,

after reading your messages, I decided to pull the whole shebang from github and build it (hopefully with the patches).  Unfortunately NO (real) success so far:

Import from embedded .kim:  
1.) Image still gets no path in the index.xml-file (but is copied correctly to the new folder on disk).
2.) The new folder is not visible in KPA (as no image references it)
3.) [is this new?] after deleting the image from the DB it is added to the blocklist (why??).  So I cannot test with the same image again (??)

Import from .kim with image in the same directory:
1.) KPA does not die and does not show any error messages ("file not found" or so) - improvement.
2.) A new folder is created and the image is copied correctly - also an improvement.
3.) no traces whatsoever in the index.xml-file !!  The image is not added to the DB and, thus, all tags are lost.  (After the next start, this image is found and added, but, alas, without any tags.)

Therefore, you fixed just on one end of the problem.  The other end (entering correct path data into the index.xml) is still as wrong as before.

Please have a second look.

Best regards, Andreas

PS: while building, I got this message:
/usr/include/exiv2/value.hpp:984:25: note: attribute for 'struct Exiv2::DateValue::Date' must follow the 'struct' keyword
         EXIV2API struct Date
                         ^ (the caret should be below the "D" in Date)
This might be trivial, but unfortunately I do not speak C++.
Comment 8 Johannes Zarl-Zierl 2014-05-11 22:07:16 UTC
Hi Andreas,

You're correct, the bug is not yet fixed (I'm still working on it). The git commit above only fixed the "low hanging fruits". (I just marked the commit to be related to this bug)

The blocklist-behaviour is not new - KPA did it this way since as long as I can remember. I'm not totally sure what's the reasoning behind this, but you can file a feature request or ask on the mailing list to change the behaviour.

Regarding the Import from .kim with image in the same directory: this should work (except the folder-bug). The kind of behaviour you describe for step 3) sounds like the database was not updated at all and the images were recognised on startup as new images. I'll keep an eye out for this while fixing the other stuff, but I might have to ask questions about that, later.

P.S.: The warning you got for /usr/include/exiv2/value.hpp is unfortunately not in our domain to fix, but it is harmless. It will be fixed in libexiv2-dev version 0.25 (see http://dev.exiv2.org/issues/859)
Comment 9 Johannes Zarl-Zierl 2014-06-25 22:56:11 UTC
I reviewed the stuff about the blocklist just yet - no, the image should not be added to the blocklist upon deletion. Sorry for the misinformation.

As for the actual bug: Sorry, I got sidetracked with other changes. Hopefully I'll get to it soon.
Comment 10 Andreas Schleth 2014-10-19 13:27:21 UTC
This has recently been reported again fpr KPA 4.5 here: https://bugs.kde.org/show_bug.cgi?id=339999 ... and it is not fixed for 4.5
Comment 11 Andreas Schleth 2014-12-15 16:30:09 UTC
After the fix for the date-bar delay problem of today, I checked:  The error persists in the latest git-revision (87a0d37).

- While in the import dialog create a new subfolder for the new images to go into
- proceed importing the images
- the folder is created, and is filled with the images (correct)
- in KPA this folder is not listed (error)
- the new images only show up after a search for "images not on disk" (error)
- in KPA these images are supposedly in the base folder of the image tree (see index.xml error)
- deleting these images does not delete the freshly imported images.
- after a restart, KPA finds the previously imported images, albeit without any of the imported tags.

What is missing here: prepend the folder name to the filename in index.xml
Comment 12 Johannes Zarl-Zierl 2014-12-15 18:51:15 UTC
*** Bug 339999 has been marked as a duplicate of this bug. ***
Comment 13 Johannes Zarl-Zierl 2014-12-24 12:31:08 UTC
Git commit 52be4e508e8cb5ed01b0a37c54d567be32be0629 by Johannes Zarl.
Committed on 24/12/2014 at 01:55.
Pushed by johanneszarl into branch 'master'.

Re-root imported images when needed.

When importing files from a .kim file into subdirectory of the image
directory, KPA now adjusts the path of the new images in the database so
that they match the imported loaction.

M  +37   -8    ImportExport/ImportHandler.cpp
M  +1    -1    ImportExport/ImportMatcher.cpp
M  +3    -0    ImportExport/ImportSettings.cpp

Comment 14 Andreas Schleth 2014-12-25 21:30:48 UTC
Hi Johannes,
thanks for the Christmas present!
I checked out the latest git master and importing now works for me (tested with my "test.kim" with 3 integrated images) - great!