Bug 806

Created attachment 1677 [details]
libraries/kipi-plugins patch

Attached proposed patch.
After applying the patch, it is also necessary to rename the files

/libraries/kipi-plugins/kipi-plugins/galleryexport/gallery.png
/libraries/kipi-plugins/kipi-plugins/sync/gallery.png

to

/libraries/kipi-plugins/kipi-plugins/galleryexport/hins-mime-gallery.png
/libraries/kipi-plugins/kipi-plugins/sync/hins-mime-gallery.png

for the patch to be completed.

The gallery.png image is not an icon and not a scalable image, so I choose a path as 'non-scalable' (hins) and the 'mime' type.

Not sure where the gallery.png image is used. If you know of it, please test and let me know :)

Created attachment 1679 [details]
Updated patch to include renaming of files

The patch works. I updated the patch to include renaming of the files (diff -urNa). Please test. :-)

I no longer see any unknown icon messages, but how do we validate the package compiled correctly?

I compared the pre and post-patch packages.

In the pre-patch version:

hicolor/non-scalable/mimetypes/gallery.png does not exist but exists in the post-patch package.

I believe the post-patch version is correct.

In the pre-patch version:

/opt/trinity/share/apps/kipiplugin_galleryexport/pics/gallery.png exists.

In the post-patch version:

/opt/trinity/share/apps/kipiplugin_galleryexport/pics/hins-mime-gallery.png exists.

Odd name in the post-patch version. Should the file name be gallery.png?

(In reply to comment #2)
> The patch works. I updated the patch to include renaming of the files (diff
> -urNa). Please test. :-)

Well done, I have learned some new here :)
I have tested the patch and it works fine.


> I no longer see any unknown icon messages, but how do we validate the package
> compiled correctly?

Good question. I don't know what use is done of the gallery.png image, so I don't know how we can test that everything is ok.
How did you validate the change for bug 804?


> I compared the pre and post-patch packages.
>...
> Odd name in the post-patch version. Should the file name be gallery.png?

The image name has changed as result of the patch. To avoid the warning message, an image needs to have a name which is made of three parts: a directory code, a type code and the image name (you can check the file admin/am_edit and look for 'dir_hash' and 'type_hash' to see how the different parts are interpreted)

>Well done, I have learned some new here :)
The tricky part I tend to forget is when applying those kinds of patches to the git tree to remember to use 'git add --all'. Otherwise the renamed files don't get merged correctly.

>How did you validate the change for bug 804?
I don't remember.

Let's see what Slavek or Tim thinks.
Worst case we can just leave everything as it is.

I noticed a bug related to the patch. Seems my final package installs gallery.png to

/hicolor/non-scalable/mimetypes/gallery.png

That is, in the root/top of the file system.

I don't know where the file should be installed. Nothing seems awry in my build script.

(In reply to comment #6)
> I noticed a bug related to the patch. Seems my final package installs
> gallery.png to
> 
> /hicolor/non-scalable/mimetypes/gallery.png
> 
> That is, in the root/top of the file system.
> 
> I don't know where the file should be installed. Nothing seems awry in my build
> script.

Hi Darrell,
on my system it is installed as:
/opt/trinity/share/apps/kipiplugin_galleryexport/pics/hins-mime-gallery.png
I don't see any file as:
/hicolor/non-scalable/mimetypes/gallery.png

But again, I am using Debian and you are using Slackware.

Perhaps you could try the patch without the changes to the file kipi-plugins/kipi-plugins/sync/Makefile.am.
There I uncommented two lines of code, but I don't see any folder 'opt/trinity/share/apps/kipiplugin_sync' in my system. Maybe it is the cause of the problem in Slackware.

PS: these days I have very little free time, so apologies if I reply late sometimes

Created attachment 1685 [details]
Updated patch to eliminate configure messages

>Perhaps you could try the patch without the changes to the file
>kipi-plugins/kipi-plugins/sync/Makefile.am.
>There I uncommented two lines of code, but I don't see any folder
>'opt/trinity/share/apps/kipiplugin_sync' in my system. Maybe it
>is the cause of the problem in Slackware.
Not a Slackware problem. The _sync directory does not exist in your or my final package because the plugin is not enabled in the configuration files and thus not compiled. Everything related to that directory is commented out. Looks like an unfinished plugin. The final configure output list of plugins does not show this _sync plugin.

I looked at this for a couple of hours. I won't pretend to understand make files. Yet I don't think the problem is a missing 'hins' option. There are two plugin directories in the final package that have standalone images and don't emit the same Unknown icon type configure error: batchprocessimages and simpleviewerexport. The main difference I noticed is those two directories have a subdirectory for the image file.

I created a new patch for galleryexport and sync directories to do likewise. Now I no longer see the Unknown icon type message and the package builds as expected. The two gallery.png image files do not need to be renamed.

I won't pretend to understand why the patch succeeds. I am only a copy cat.

I tried once to uncomment the sync directory and compile but no luck. Hence my comment of an unfinished plugin.

Please review and test the patch when you have time.

Hi Darrell,
I like your solution better than mine. I couldn't test it yet, but I looked at the code and everything seems fine to be.
If I can I will test tomorrow, I should have some free time if nothing unexpected happens

(In reply to comment #9)
> Hi Darrell,
> I like your solution better than mine. I couldn't test it yet, but I looked at
> the code and everything seems fine to be.
> If I can I will test tomorrow, I should have some free time if nothing
> unexpected happens

Darrell, I tested the patch and everything looks ok. As I said, I think your solution is more elegant than mine. I think you can push it to GIT.

The patch in attachment 1685 [details] was pushed to git in commit 23cbafd2.