soprano-devel Mailing List for Soprano

Hello!

When you have decided to make a release? Just interesting.

Best regards, Denis Kuplyakov.

2015-07-27 16:53 GMT+03:00 Денис Купляков <den...@gm...>:

> I don't know concrete date. The situation is that Calligra Author hardly
> depends on RDF with the new functionality (outliner). But the fact is that
> this outliner isn't finished, so I with calligra devs have decided by now
> to make RDF not to be a hard dependency and hide Outliner from users if
> there is no valid Soprano found during building of code. So I can say we
> will need new soprano released with a release of Author that will have a
> first version of Outliner. Outliner is my GSoC project and is planned to be
> finished at least by end of september, if there will be no other underwater
> stones (that was my previous GSoC too, but I didn't finished with it).
>
> And Friedrich W. H. Kossebau (calligra dev) recommends some renaming:
>
> Problem:
>
> macro_optional_find_package(Soprano)
>
> finds the qt4 version here. Which of course is the wrong version.
>
> (Uses the FindSoprano.cmake as installed with the kdelibs4 devel package)
>
> Best solution, very recommeded to invest into that:
>
> have soprano-qt5 install some cmake config files, and add a -qt5 postfix
> to the name, like done with other qt-using software for their qt5 variant.
> Then change the package find line to
>
> macro_optional_find_package(Soprano-qt5)
>
> or whatever name you will use for this variant.
>
> See http://api.kde.org/ecm/module/ECMPackageConfigHelpers.html for
> related macros to use with soprano perhaps, and see e.g. in kdesupport
> modules how they are used, e.g. in attica:
>
> http://quickgit.kde.org/?p=attica.git&a=tree, files
> KF5AtticaConfig.cmake.in and CMakeLists.txt
>
>
> Best regards, Denis Kuplyakov.
>
> 2015-07-27 16:41 GMT+03:00 Vishesh Handa <me...@vh...>:
>
>> Sure. Makes sense. It needs to be done.
>>
>> Do you want to give a date by which you would like this done? I'm
>> putting it on my todo list.
>> Vishesh Handa
>>
>>
>> On Wed, Jul 22, 2015 at 6:52 PM, Денис Купляков <den...@gm...>
>> wrote:
>> > Hello Vishesh,
>> >
>> > My propose for now is:
>> > 1) applying my patch,
>> > 2) we are writing README.PACKAGERS to make situation according
>> > QT5_BUILD flag clear,
>> > 3) I don't think my patch needs any serious testing, it is a simple
>> > addition of plugin metainfo (also I should notice that Qt5 builded
>> > version on my setup passing more unittests than Qt4 version does)
>> > 4) announcing a new 2.(x+1) release with a single change: Qt5 support.
>> > I can make a post on planet KDE. Also some "release mail" on this list
>> > will be fine.
>> >
>> > What do you think? I need sometime for Soprano to live, cuz porting
>> > RDF support of Calligra to another library requires a big effort.
>> >
>> > Best regards, Denis Kuplyakov.
>> >
>> > 2015-07-18 13:34 GMT+03:00 Vishesh Handa <me...@vh...>:
>> >> Hey Денис
>> >>
>> >> On Wed, Jul 15, 2015 at 8:25 PM, Денис Купляков <den...@gm...>
>> wrote:
>> >>> What is a status of the project? Could we in Calligra be sure that at
>> >>> least this patch will get into the release and we will be able to
>> >>> access Soprano qt5 version in distros?
>> >>
>> >> The project hasn't seen any commits since Oct 2013, and even then it
>> >> was only getting updates required for Nepomuk. When I took over
>> >> Nepomuk, I guess I also took over Soprano.
>> >>
>> >> The Soprano code actually lives on the kde servers, so if you're a kde
>> >> developer you can just push the code there.
>> >>
>> >> Regarding making a new release, I'm not sure what the process would
>> >> be. Should I be releasing Soprano 3 or continuing 2.x? I'm no longer
>> >> using Soprano in any personal projects, so I don't even test it out.
>> >>
>> >> --
>> >> Vishesh Handa
>>
>
>

Sure. Makes sense. It needs to be done.

Do you want to give a date by which you would like this done? I'm
putting it on my todo list.
Vishesh Handa


On Wed, Jul 22, 2015 at 6:52 PM, Денис Купляков <den...@gm...> wrote:
> Hello Vishesh,
>
> My propose for now is:
> 1) applying my patch,
> 2) we are writing README.PACKAGERS to make situation according
> QT5_BUILD flag clear,
> 3) I don't think my patch needs any serious testing, it is a simple
> addition of plugin metainfo (also I should notice that Qt5 builded
> version on my setup passing more unittests than Qt4 version does)
> 4) announcing a new 2.(x+1) release with a single change: Qt5 support.
> I can make a post on planet KDE. Also some "release mail" on this list
> will be fine.
>
> What do you think? I need sometime for Soprano to live, cuz porting
> RDF support of Calligra to another library requires a big effort.
>
> Best regards, Denis Kuplyakov.
>
> 2015-07-18 13:34 GMT+03:00 Vishesh Handa <me...@vh...>:
>> Hey Денис
>>
>> On Wed, Jul 15, 2015 at 8:25 PM, Денис Купляков <den...@gm...> wrote:
>>> What is a status of the project? Could we in Calligra be sure that at
>>> least this patch will get into the release and we will be able to
>>> access Soprano qt5 version in distros?
>>
>> The project hasn't seen any commits since Oct 2013, and even then it
>> was only getting updates required for Nepomuk. When I took over
>> Nepomuk, I guess I also took over Soprano.
>>
>> The Soprano code actually lives on the kde servers, so if you're a kde
>> developer you can just push the code there.
>>
>> Regarding making a new release, I'm not sure what the process would
>> be. Should I be releasing Soprano 3 or continuing 2.x? I'm no longer
>> using Soprano in any personal projects, so I don't even test it out.
>>
>> --
>> Vishesh Handa

I don't know concrete date. The situation is that Calligra Author hardly
depends on RDF with the new functionality (outliner). But the fact is that
this outliner isn't finished, so I with calligra devs have decided by now
to make RDF not to be a hard dependency and hide Outliner from users if
there is no valid Soprano found during building of code. So I can say we
will need new soprano released with a release of Author that will have a
first version of Outliner. Outliner is my GSoC project and is planned to be
finished at least by end of september, if there will be no other underwater
stones (that was my previous GSoC too, but I didn't finished with it).

And Friedrich W. H. Kossebau (calligra dev) recommends some renaming:

Problem:

macro_optional_find_package(Soprano)

finds the qt4 version here. Which of course is the wrong version.

(Uses the FindSoprano.cmake as installed with the kdelibs4 devel package)

Best solution, very recommeded to invest into that:

have soprano-qt5 install some cmake config files, and add a -qt5 postfix to
the name, like done with other qt-using software for their qt5 variant.
Then change the package find line to

macro_optional_find_package(Soprano-qt5)

or whatever name you will use for this variant.

See http://api.kde.org/ecm/module/ECMPackageConfigHelpers.html for related
macros to use with soprano perhaps, and see e.g. in kdesupport modules how
they are used, e.g. in attica:

http://quickgit.kde.org/?p=attica.git&a=tree, files KF5AtticaConfig.cmake.in
 and CMakeLists.txt


Best regards, Denis Kuplyakov.

2015-07-27 16:41 GMT+03:00 Vishesh Handa <me...@vh...>:

> Sure. Makes sense. It needs to be done.
>
> Do you want to give a date by which you would like this done? I'm
> putting it on my todo list.
> Vishesh Handa
>
>
> On Wed, Jul 22, 2015 at 6:52 PM, Денис Купляков <den...@gm...>
> wrote:
> > Hello Vishesh,
> >
> > My propose for now is:
> > 1) applying my patch,
> > 2) we are writing README.PACKAGERS to make situation according
> > QT5_BUILD flag clear,
> > 3) I don't think my patch needs any serious testing, it is a simple
> > addition of plugin metainfo (also I should notice that Qt5 builded
> > version on my setup passing more unittests than Qt4 version does)
> > 4) announcing a new 2.(x+1) release with a single change: Qt5 support.
> > I can make a post on planet KDE. Also some "release mail" on this list
> > will be fine.
> >
> > What do you think? I need sometime for Soprano to live, cuz porting
> > RDF support of Calligra to another library requires a big effort.
> >
> > Best regards, Denis Kuplyakov.
> >
> > 2015-07-18 13:34 GMT+03:00 Vishesh Handa <me...@vh...>:
> >> Hey Денис
> >>
> >> On Wed, Jul 15, 2015 at 8:25 PM, Денис Купляков <den...@gm...>
> wrote:
> >>> What is a status of the project? Could we in Calligra be sure that at
> >>> least this patch will get into the release and we will be able to
> >>> access Soprano qt5 version in distros?
> >>
> >> The project hasn't seen any commits since Oct 2013, and even then it
> >> was only getting updates required for Nepomuk. When I took over
> >> Nepomuk, I guess I also took over Soprano.
> >>
> >> The Soprano code actually lives on the kde servers, so if you're a kde
> >> developer you can just push the code there.
> >>
> >> Regarding making a new release, I'm not sure what the process would
> >> be. Should I be releasing Soprano 3 or continuing 2.x? I'm no longer
> >> using Soprano in any personal projects, so I don't even test it out.
> >>
> >> --
> >> Vishesh Handa
>

Hello Vishesh,

My propose for now is:
1) applying my patch,
2) we are writing README.PACKAGERS to make situation according
QT5_BUILD flag clear,
3) I don't think my patch needs any serious testing, it is a simple
addition of plugin metainfo (also I should notice that Qt5 builded
version on my setup passing more unittests than Qt4 version does)
4) announcing a new 2.(x+1) release with a single change: Qt5 support.
I can make a post on planet KDE. Also some "release mail" on this list
will be fine.

What do you think? I need sometime for Soprano to live, cuz porting
RDF support of Calligra to another library requires a big effort.

Best regards, Denis Kuplyakov.

2015-07-18 13:34 GMT+03:00 Vishesh Handa <me...@vh...>:
> Hey Денис
>
> On Wed, Jul 15, 2015 at 8:25 PM, Денис Купляков <den...@gm...> wrote:
>> What is a status of the project? Could we in Calligra be sure that at
>> least this patch will get into the release and we will be able to
>> access Soprano qt5 version in distros?
>
> The project hasn't seen any commits since Oct 2013, and even then it
> was only getting updates required for Nepomuk. When I took over
> Nepomuk, I guess I also took over Soprano.
>
> The Soprano code actually lives on the kde servers, so if you're a kde
> developer you can just push the code there.
>
> Regarding making a new release, I'm not sure what the process would
> be. Should I be releasing Soprano 3 or continuing 2.x? I'm no longer
> using Soprano in any personal projects, so I don't even test it out.
>
> --
> Vishesh Handa

Hello!

I'm developing Calligra Author app now and it is relying on Soprano
functionality for RDF handling. As you know Calligra and KDE is moving
to KF5, so Qt5 version of Soprano is needed. I've found that it
already has a special CMake flag and is easily builded on qt5
environment. The only thing wasn't working for me were plugin loading.
I've added some Q_PLUGIN_METADATA macroses around the code and Soprano
is working with Author in KF5 environment! Thanks! I'm attaching a
patch to this email.

What is a status of the project? Could we in Calligra be sure that at
least this patch will get into the release and we will be able to
access Soprano qt5 version in distros?

Best regards, Denis Kuplyakov.

On Monday 28 October 2013 09:38:21 Lisandro Damián Nicanor Pérez Meyer wrote:
> On Monday 28 October 2013 10:47:13 Vishesh Handa wrote:
> > Hey everyone
> > 
> > The patch applies perfectly (in the 2.9 branch). However, my cmake foo
> > is not the great, so it would be nice if some else could also take a
> > look at it.
> 
> Great :)
> 
> > @Lisandro: I assume you'll want a new release of Soprano to be made?
> > 2.9.5? Soprano 2.10 is not going to be released, we're going to jump
> > to version 3.0 after this which will break binary compatibility since
> > we will be moving to Qt5.
> 
> Sune Vuorela told me that there might be soprano upstream devs that might be
> using our packaged virtuoso. If we switch virtuoso+soprano to m-a paths,
> their dev builds of soprano will stop working.
> 
> So if the patch gets accepted upstream and you find it ok, we can simply
> patch our current soprano (2.9.3 iirc) and upstream devs will get it the
> necessary changes via soprano's repo.

OK, due to various reasons we need to move forward, so we will apply the 
patch. If you can make a 2.9.5 release, the better :-)

Kinds regards, Lisandro.

-- 

Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

On Monday 28 October 2013 10:47:13 Vishesh Handa wrote:
> Hey everyone
> 
> The patch applies perfectly (in the 2.9 branch). However, my cmake foo
> is not the great, so it would be nice if some else could also take a
> look at it.

Great :)
 
> @Lisandro: I assume you'll want a new release of Soprano to be made?
> 2.9.5? Soprano 2.10 is not going to be released, we're going to jump
> to version 3.0 after this which will break binary compatibility since
> we will be moving to Qt5.

Sune Vuorela told me that there might be soprano upstream devs that might be 
using our packaged virtuoso. If we switch virtuoso+soprano to m-a paths, their 
dev builds of soprano will stop working.

So if the patch gets accepted upstream and you find it ok, we can simply patch 
our current soprano (2.9.3 iirc) and upstream devs will get it the necessary 
changes via soprano's repo.


Kinds regards, Lisandro.

-- 
"In college, I cooked some hot dogs by putting metal forks in each end of the
hot dog and running 120 volts through it. Hot dogs have just enough
conductivity so that this works well"
  greenroom(576281) - on a truly geeky way to cook hot dogs.
  Posted in Slashdot, also found in The Open Source Cookbook for Geeks.

Lisandro Damián Nicanor Pérez Meyer
http://perezmeyer.com.ar/
http://perezmeyer.blogspot.com/

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111813/#review37772
-----------------------------------------------------------

Ship it!


:)

- Vishesh Handa


On Aug. 1, 2013, 5:57 a.m., Michael Palimaka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111813/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 5:57 a.m.)
> 
> 
> Review request for Soprano.
> 
> 
> Description
> -------
> 
> Not all systems provide a separate libdl, which causes soprano to fail to build on FreeBSD. This change replaces the hardcoded library with a CMake variable that is filled appropriately based on the system.
> 
> 
> Diffs
> -----
> 
>   cmake/modules/FindRedland.cmake c7274c981333c858ae0c077ef819f8d445cbd758 
> 
> Diff: http://git.reviewboard.kde.org/r/111813/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Palimaka
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111813/#review37758
-----------------------------------------------------------


Dummy comment because it looks like this request never made it to the mailing list.

- Michael Palimaka


On Aug. 1, 2013, 5:57 a.m., Michael Palimaka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111813/
> -----------------------------------------------------------
> 
> (Updated Aug. 1, 2013, 5:57 a.m.)
> 
> 
> Review request for Soprano.
> 
> 
> Description
> -------
> 
> Not all systems provide a separate libdl, which causes soprano to fail to build on FreeBSD. This change replaces the hardcoded library with a CMake variable that is filled appropriately based on the system.
> 
> 
> Diffs
> -----
> 
>   cmake/modules/FindRedland.cmake c7274c981333c858ae0c077ef819f8d445cbd758 
> 
> Diff: http://git.reviewboard.kde.org/r/111813/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Michael Palimaka
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/
-----------------------------------------------------------

(Updated June 12, 2013, 12:12 a.m.)


Status
------

This change has been marked as submitted.


Review request for Soprano and Vishesh Handa.


Description
-------

    Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
    giving the last error in each thread.
    
    Change the implementation so clearError() calls remove(), instead
    of setting the error to a default value. This prevents the hash
    growing endlessly. It is still possible for error values to leak,
    but only when someone forgets to call clearError() before a thread
     finishes (and only when an actual error occurs).
    
    BUG:320123
    REVIEW: 110897


This addresses bugs 269442 and 320123.
    http://bugs.kde.org/show_bug.cgi?id=269442
    http://bugs.kde.org/show_bug.cgi?id=320123


Diffs
-----

  soprano/error.cpp bff34dd 

Diff: http://git.reviewboard.kde.org/r/110897/diff/


Testing
-------

Compiled, ran


Thanks,

Simeon Bird

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/#review34196
-----------------------------------------------------------


This review has been submitted with commit 15748cd9e64c351e0bebcc8907d5a078cd0c4f88 by Simeon Bird to branch 2.9.

- Commit Hook


On June 11, 2013, 10:43 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110897/
> -----------------------------------------------------------
> 
> (Updated June 11, 2013, 10:43 p.m.)
> 
> 
> Review request for Soprano and Vishesh Handa.
> 
> 
> Description
> -------
> 
>     Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
>     giving the last error in each thread.
>     
>     Change the implementation so clearError() calls remove(), instead
>     of setting the error to a default value. This prevents the hash
>     growing endlessly. It is still possible for error values to leak,
>     but only when someone forgets to call clearError() before a thread
>      finishes (and only when an actual error occurs).
>     
>     BUG:320123
>     REVIEW: 110897
> 
> 
> This addresses bugs 269442 and 320123.
>     http://bugs.kde.org/show_bug.cgi?id=269442
>     http://bugs.kde.org/show_bug.cgi?id=320123
> 
> 
> Diffs
> -----
> 
>   soprano/error.cpp bff34dd 
> 
> Diff: http://git.reviewboard.kde.org/r/110897/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/#review34195
-----------------------------------------------------------

Ship it!


Ship It!

- Vishesh Handa


On June 11, 2013, 10:43 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110897/
> -----------------------------------------------------------
> 
> (Updated June 11, 2013, 10:43 p.m.)
> 
> 
> Review request for Soprano and Vishesh Handa.
> 
> 
> Description
> -------
> 
>     Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
>     giving the last error in each thread.
>     
>     Change the implementation so clearError() calls remove(), instead
>     of setting the error to a default value. This prevents the hash
>     growing endlessly. It is still possible for error values to leak,
>     but only when someone forgets to call clearError() before a thread
>      finishes (and only when an actual error occurs).
>     
>     BUG:320123
>     REVIEW: 110897
> 
> 
> This addresses bugs 269442 and 320123.
>     http://bugs.kde.org/show_bug.cgi?id=269442
>     http://bugs.kde.org/show_bug.cgi?id=320123
> 
> 
> Diffs
> -----
> 
>   soprano/error.cpp bff34dd 
> 
> Diff: http://git.reviewboard.kde.org/r/110897/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/
-----------------------------------------------------------

(Updated June 11, 2013, 10:43 p.m.)


Review request for Soprano and Vishesh Handa.


Changes
-------

Upload the right file this time


Description
-------

    Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
    giving the last error in each thread.
    
    Change the implementation so clearError() calls remove(), instead
    of setting the error to a default value. This prevents the hash
    growing endlessly. It is still possible for error values to leak,
    but only when someone forgets to call clearError() before a thread
     finishes (and only when an actual error occurs).
    
    BUG:320123
    REVIEW: 110897


This addresses bugs 269442 and 320123.
    http://bugs.kde.org/show_bug.cgi?id=269442
    http://bugs.kde.org/show_bug.cgi?id=320123


Diffs (updated)
-----

  soprano/error.cpp bff34dd 

Diff: http://git.reviewboard.kde.org/r/110897/diff/


Testing
-------

Compiled, ran


Thanks,

Simeon Bird

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/
-----------------------------------------------------------

(Updated June 11, 2013, 10:42 p.m.)


Review request for Soprano and Vishesh Handa.


Changes
-------

Much easier way to prevent the ErrorCache from leaking - note that before it would not leak until there was an error, but once it did, it would leak for every thread that executed, even the successful ones.


Summary (updated)
-----------------

Fix memory leak in ErrorCache


Description (updated)
-------

    Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
    giving the last error in each thread.
    
    Change the implementation so clearError() calls remove(), instead
    of setting the error to a default value. This prevents the hash
    growing endlessly. It is still possible for error values to leak,
    but only when someone forgets to call clearError() before a thread
     finishes (and only when an actual error occurs).
    
    BUG:320123
    REVIEW: 110897


This addresses bugs 269442 and 320123.
    http://bugs.kde.org/show_bug.cgi?id=269442
    http://bugs.kde.org/show_bug.cgi?id=320123


Diffs (updated)
-----

  soprano/error.h 12d233e 
  soprano/error.cpp bff34dd 

Diff: http://git.reviewboard.kde.org/r/110897/diff/


Testing
-------

Compiled, ran


Thanks,

Simeon Bird

> On June 8, 2013, 7:10 p.m., Vishesh Handa wrote:
> > Yes for (1), no for (2).
> > 
> > And maybe no for (1). Ideally (1) should be fixed by qDebug not printing these messages when Soprano is compiled in release mode or something like that. I remember reading a bug report about this on bugs.kde.org by Thiago. Maybe you should take a look at that. Otherwise commit (1), I guess.
> > 
> > I'm really not too fond of the ErrorCache's design because doing this hash lookup is not especially cheap, specially when it is done all over. That being said, it makes sense in the way it is being used in Soprano. Lets take the example of the Soprano::Model -
> > 
> > Two different threads can access two member functions of the same model and can return errors.
> > 
> > Thread 1 - Soprano::Model::executeQuery( Query1 );
> > Thread 2 - Soprano::Model::executeQuery( Query2 );
> > 
> > Thread 1 will want to check the errors, and so will thread 2, if both of these operations are happening somewhere around the same time, Thread1 could get Thread2's error and vice versa.
> > 
> > That being said - we need to find a way that the QHash does not just keep growing. I'm open to suggestions.
> > 
> > PS: Nice work with your patches!
> 
> Simeon Bird wrote:
>     So, er, actually, I checked and according to the documentation http://techbase.kde.org/Development/CMake/Addons_for_KDE#Buildtypes qDebug should *already* not print messages when Soprano is compiled in release mode. 
>     Is this actually the case for Soprano? If not, do we know why not? 
>     
>     Anyway, given that I'll shelve that commit for now.
>     
>     As to 2 - what actually is the reason for not using C++ exceptions here? I mean, you don't really want a per-thread error store, you really want extended error information from each function, which is what C++ exceptions are for. Is it just that the external interface is now in terms of setError and can't be changed, or was there some other design reason initially?

The only reason for not using exceptions was that at the time of the implementation Qt was typically compiled without exceptions enabled. Back then, I thought it would be best to not use exceptions in Qt-based code at all. If I were to rewrite it today I would probably use exceptions instead because in the end the ErrorCache is just a poor-man's exception system.

Using exceptions would be yet another point for the good old Soprano 3 todo list (http://soprano.sourceforge.net/node/39).


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/#review33956
-----------------------------------------------------------


On June 8, 2013, 7:03 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110897/
> -----------------------------------------------------------
> 
> (Updated June 8, 2013, 7:03 p.m.)
> 
> 
> Review request for Soprano and Vishesh Handa.
> 
> 
> Description
> -------
> 
> commit a7e2643a0fd6bd3608bafc6ea972b4b5d143e0a7
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 14:16:29 2013 -0400
> 
>     Do not print the last error that soprano saw to the console. All this
>     does is fill up .xsession-errors. The caller can print the message
>     themselves if they want.
>     
>     BUG: 269442
> 
> commit 0f9dc7a6f434efd58ed21c99bc1cf280c2fb6e4e
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 13:58:01 2013 -0400
> 
>     Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
>     giving the last error in each thread.
>     
>     Once a thread has exited, there is no way to access or modify its error.
>     Since all actual work is done inside temporary threads, this is
>     not a sensible thing to do, and means that over time the QHash can
>     grow an infinite number of buckets, until it crashes.
>     
>     What the class seems actually meant to do is to pass error messages from
>     temporary threads back to the caller. Therefore, remove the per-thread
>     hash and just store the last error that occurred for this object
>     (ie, make it a per-object errno).
>     
>     BUG: 320123
> 
> 
> This addresses bugs 269442 and 320123.
>     http://bugs.kde.org/show_bug.cgi?id=269442
>     http://bugs.kde.org/show_bug.cgi?id=320123
> 
> 
> Diffs
> -----
> 
>   soprano/error.cpp bff34dd 
>   soprano/error.h 12d233e 
> 
> Diff: http://git.reviewboard.kde.org/r/110897/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

> On June 8, 2013, 7:10 p.m., Vishesh Handa wrote:
> > Yes for (1), no for (2).
> > 
> > And maybe no for (1). Ideally (1) should be fixed by qDebug not printing these messages when Soprano is compiled in release mode or something like that. I remember reading a bug report about this on bugs.kde.org by Thiago. Maybe you should take a look at that. Otherwise commit (1), I guess.
> > 
> > I'm really not too fond of the ErrorCache's design because doing this hash lookup is not especially cheap, specially when it is done all over. That being said, it makes sense in the way it is being used in Soprano. Lets take the example of the Soprano::Model -
> > 
> > Two different threads can access two member functions of the same model and can return errors.
> > 
> > Thread 1 - Soprano::Model::executeQuery( Query1 );
> > Thread 2 - Soprano::Model::executeQuery( Query2 );
> > 
> > Thread 1 will want to check the errors, and so will thread 2, if both of these operations are happening somewhere around the same time, Thread1 could get Thread2's error and vice versa.
> > 
> > That being said - we need to find a way that the QHash does not just keep growing. I'm open to suggestions.
> > 
> > PS: Nice work with your patches!

So, er, actually, I checked and according to the documentation http://techbase.kde.org/Development/CMake/Addons_for_KDE#Buildtypes qDebug should *already* not print messages when Soprano is compiled in release mode. 
Is this actually the case for Soprano? If not, do we know why not? 

Anyway, given that I'll shelve that commit for now.

As to 2 - what actually is the reason for not using C++ exceptions here? I mean, you don't really want a per-thread error store, you really want extended error information from each function, which is what C++ exceptions are for. Is it just that the external interface is now in terms of setError and can't be changed, or was there some other design reason initially?


- Simeon


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/#review33956
-----------------------------------------------------------


On June 8, 2013, 7:03 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110897/
> -----------------------------------------------------------
> 
> (Updated June 8, 2013, 7:03 p.m.)
> 
> 
> Review request for Soprano and Vishesh Handa.
> 
> 
> Description
> -------
> 
> commit a7e2643a0fd6bd3608bafc6ea972b4b5d143e0a7
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 14:16:29 2013 -0400
> 
>     Do not print the last error that soprano saw to the console. All this
>     does is fill up .xsession-errors. The caller can print the message
>     themselves if they want.
>     
>     BUG: 269442
> 
> commit 0f9dc7a6f434efd58ed21c99bc1cf280c2fb6e4e
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 13:58:01 2013 -0400
> 
>     Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
>     giving the last error in each thread.
>     
>     Once a thread has exited, there is no way to access or modify its error.
>     Since all actual work is done inside temporary threads, this is
>     not a sensible thing to do, and means that over time the QHash can
>     grow an infinite number of buckets, until it crashes.
>     
>     What the class seems actually meant to do is to pass error messages from
>     temporary threads back to the caller. Therefore, remove the per-thread
>     hash and just store the last error that occurred for this object
>     (ie, make it a per-object errno).
>     
>     BUG: 320123
> 
> 
> This addresses bugs 269442 and 320123.
>     http://bugs.kde.org/show_bug.cgi?id=269442
>     http://bugs.kde.org/show_bug.cgi?id=320123
> 
> 
> Diffs
> -----
> 
>   soprano/error.cpp bff34dd 
>   soprano/error.h 12d233e 
> 
> Diff: http://git.reviewboard.kde.org/r/110897/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/#review33956
-----------------------------------------------------------


Yes for (1), no for (2).

And maybe no for (1). Ideally (1) should be fixed by qDebug not printing these messages when Soprano is compiled in release mode or something like that. I remember reading a bug report about this on bugs.kde.org by Thiago. Maybe you should take a look at that. Otherwise commit (1), I guess.

I'm really not too fond of the ErrorCache's design because doing this hash lookup is not especially cheap, specially when it is done all over. That being said, it makes sense in the way it is being used in Soprano. Lets take the example of the Soprano::Model -

Two different threads can access two member functions of the same model and can return errors.

Thread 1 - Soprano::Model::executeQuery( Query1 );
Thread 2 - Soprano::Model::executeQuery( Query2 );

Thread 1 will want to check the errors, and so will thread 2, if both of these operations are happening somewhere around the same time, Thread1 could get Thread2's error and vice versa.

That being said - we need to find a way that the QHash does not just keep growing. I'm open to suggestions.

PS: Nice work with your patches!

- Vishesh Handa


On June 8, 2013, 7:03 p.m., Simeon Bird wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/110897/
> -----------------------------------------------------------
> 
> (Updated June 8, 2013, 7:03 p.m.)
> 
> 
> Review request for Soprano and Vishesh Handa.
> 
> 
> Description
> -------
> 
> commit a7e2643a0fd6bd3608bafc6ea972b4b5d143e0a7
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 14:16:29 2013 -0400
> 
>     Do not print the last error that soprano saw to the console. All this
>     does is fill up .xsession-errors. The caller can print the message
>     themselves if they want.
>     
>     BUG: 269442
> 
> commit 0f9dc7a6f434efd58ed21c99bc1cf280c2fb6e4e
> Author: Simeon Bird <sp...@ia...>
> Date:   Sat Jun 8 13:58:01 2013 -0400
> 
>     Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
>     giving the last error in each thread.
>     
>     Once a thread has exited, there is no way to access or modify its error.
>     Since all actual work is done inside temporary threads, this is
>     not a sensible thing to do, and means that over time the QHash can
>     grow an infinite number of buckets, until it crashes.
>     
>     What the class seems actually meant to do is to pass error messages from
>     temporary threads back to the caller. Therefore, remove the per-thread
>     hash and just store the last error that occurred for this object
>     (ie, make it a per-object errno).
>     
>     BUG: 320123
> 
> 
> This addresses bugs 269442 and 320123.
>     http://bugs.kde.org/show_bug.cgi?id=269442
>     http://bugs.kde.org/show_bug.cgi?id=320123
> 
> 
> Diffs
> -----
> 
>   soprano/error.cpp bff34dd 
>   soprano/error.h 12d233e 
> 
> Diff: http://git.reviewboard.kde.org/r/110897/diff/
> 
> 
> Testing
> -------
> 
> Compiled, ran
> 
> 
> Thanks,
> 
> Simeon Bird
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/110897/
-----------------------------------------------------------

Review request for Soprano and Vishesh Handa.


Description
-------

commit a7e2643a0fd6bd3608bafc6ea972b4b5d143e0a7
Author: Simeon Bird <sp...@ia...>
Date:   Sat Jun 8 14:16:29 2013 -0400

    Do not print the last error that soprano saw to the console. All this
    does is fill up .xsession-errors. The caller can print the message
    themselves if they want.
    
    BUG: 269442

commit 0f9dc7a6f434efd58ed21c99bc1cf280c2fb6e4e
Author: Simeon Bird <sp...@ia...>
Date:   Sat Jun 8 13:58:01 2013 -0400

    Soprano::Error::ErrorCache stores a QHash <QThread*, Error>
    giving the last error in each thread.
    
    Once a thread has exited, there is no way to access or modify its error.
    Since all actual work is done inside temporary threads, this is
    not a sensible thing to do, and means that over time the QHash can
    grow an infinite number of buckets, until it crashes.
    
    What the class seems actually meant to do is to pass error messages from
    temporary threads back to the caller. Therefore, remove the per-thread
    hash and just store the last error that occurred for this object
    (ie, make it a per-object errno).
    
    BUG: 320123


This addresses bugs 269442 and 320123.
    http://bugs.kde.org/show_bug.cgi?id=269442
    http://bugs.kde.org/show_bug.cgi?id=320123


Diffs
-----

  soprano/error.cpp bff34dd 
  soprano/error.h 12d233e 

Diff: http://git.reviewboard.kde.org/r/110897/diff/


Testing
-------

Compiled, ran


Thanks,

Simeon Bird

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106471/
-----------------------------------------------------------

(Updated May 28, 2013, 3:41 p.m.)


Status
------

This change has been marked as submitted.


Review request for Nepomuk, Soprano and Sebastian Trueg.


Description
-------

    Optimize the N3 conversion by escaping all the necessary characters in
    one go, and by caching the QString version of the data type.
    
    They aren't that many data types, so this shouldn't take up too much
    space.


Diffs
-----

  soprano/node.cpp 8ebbe0e 

Diff: http://git.reviewboard.kde.org/r/106471/diff/


Testing
-------


Thanks,

Vishesh Handa

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106467/
-----------------------------------------------------------

(Updated May 28, 2013, 3:24 p.m.)


Status
------

This change has been marked as submitted.


Review request for Nepomuk, Soprano and Sebastian Trueg.


Description
-------

    Only use 1 SQLFetchData command in most of the cases.
    
    Callgrind stats show that 67.5% of the time in this function is spent in
    the first SQLFetchData, and an additional 27% in the second SQLGetData.
    We can avoid some of this extra cost, by only calling the function
    once.

I can change the size of the default buffer if required.


Diffs
-----

  backends/virtuoso/odbcqueryresult.cpp a4f2387 

Diff: http://git.reviewboard.kde.org/r/106467/diff/


Testing
-------


Thanks,

Vishesh Handa

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106467/#review33298
-----------------------------------------------------------


This review has been submitted with commit f8747bc9d8fa916907d90b7f2bf84ffba923becb by Vishesh Handa to branch 2.9.

- Commit Hook


On Sept. 17, 2012, 9:37 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106467/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2012, 9:37 a.m.)
> 
> 
> Review request for Nepomuk, Soprano and Sebastian Trueg.
> 
> 
> Description
> -------
> 
>     Only use 1 SQLFetchData command in most of the cases.
>     
>     Callgrind stats show that 67.5% of the time in this function is spent in
>     the first SQLFetchData, and an additional 27% in the second SQLGetData.
>     We can avoid some of this extra cost, by only calling the function
>     once.
> 
> I can change the size of the default buffer if required.
> 
> 
> Diffs
> -----
> 
>   backends/virtuoso/odbcqueryresult.cpp a4f2387 
> 
> Diff: http://git.reviewboard.kde.org/r/106467/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106467/#review33136
-----------------------------------------------------------



backends/virtuoso/odbcqueryresult.cpp
<http://git.reviewboard.kde.org/r/106467/#comment24505>

    The problem was that this should have had a +1 in the end to account for the null character.
    
    Tested extensively. It now works. I'll be committing.
    
    This gives a 20-23% performance upgrade. Tested by fetching 50000 quads.


- Vishesh Handa


On Sept. 17, 2012, 9:37 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106467/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2012, 9:37 a.m.)
> 
> 
> Review request for Nepomuk, Soprano and Sebastian Trueg.
> 
> 
> Description
> -------
> 
>     Only use 1 SQLFetchData command in most of the cases.
>     
>     Callgrind stats show that 67.5% of the time in this function is spent in
>     the first SQLFetchData, and an additional 27% in the second SQLGetData.
>     We can avoid some of this extra cost, by only calling the function
>     once.
> 
> I can change the size of the default buffer if required.
> 
> 
> Diffs
> -----
> 
>   backends/virtuoso/odbcqueryresult.cpp a4f2387 
> 
> Diff: http://git.reviewboard.kde.org/r/106467/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109192/
-----------------------------------------------------------

(Updated May 11, 2013, 8:54 p.m.)


Status
------

This change has been marked as submitted.


Review request for Soprano, Sebastian Trueg and Vishesh Handa.


Description
-------

On Windows, different calling conventions let developers suffer and applications crash. In this case, soprano should care about the calling convention used in librdf.


Diffs
-----

  backends/redland/redlandworld.cpp 3c6facd 

Diff: http://git.reviewboard.kde.org/r/109192/diff/


Testing
-------

Windows. Please test on Linux.


Thanks,

Patrick Spendrin

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106467/#review32173
-----------------------------------------------------------



backends/virtuoso/odbcqueryresult.cpp
<http://git.reviewboard.kde.org/r/106467/#comment23955>

    Reading the code seems like you need a -4.
    
    int r = SQLGetData( d->m_hstmt, colNum, SQL_C_CHAR, *buffer, bufSize-4, length );



backends/virtuoso/odbcqueryresult.cpp
<http://git.reviewboard.kde.org/r/106467/#comment23956>

    And here the same:
    
    if( *length > bufSize-4) {


- Ignacio Serantes


On Sept. 17, 2012, 11:37 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106467/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2012, 11:37 a.m.)
> 
> 
> Review request for Nepomuk, Soprano and Sebastian Trueg.
> 
> 
> Description
> -------
> 
>     Only use 1 SQLFetchData command in most of the cases.
>     
>     Callgrind stats show that 67.5% of the time in this function is spent in
>     the first SQLFetchData, and an additional 27% in the second SQLGetData.
>     We can avoid some of this extra cost, by only calling the function
>     once.
> 
> I can change the size of the default buffer if required.
> 
> 
> Diffs
> -----
> 
>   backends/virtuoso/odbcqueryresult.cpp a4f2387 
> 
> Diff: http://git.reviewboard.kde.org/r/106467/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>

> On Feb. 8, 2013, 6:33 p.m., Albert Astals Cid wrote:
> > What's the status of this? Should it be commited or not? If not maybe we should discarded to get a clearer queue in reviewboard?

The patch seems to be slightly buggy and often skips one character. It needs to be tested more, and some proper benchmarks need to be performed.


- Vishesh


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/106467/#review26996
-----------------------------------------------------------


On Sept. 17, 2012, 9:37 a.m., Vishesh Handa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106467/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2012, 9:37 a.m.)
> 
> 
> Review request for Nepomuk, Soprano and Sebastian Trueg.
> 
> 
> Description
> -------
> 
>     Only use 1 SQLFetchData command in most of the cases.
>     
>     Callgrind stats show that 67.5% of the time in this function is spent in
>     the first SQLFetchData, and an additional 27% in the second SQLGetData.
>     We can avoid some of this extra cost, by only calling the function
>     once.
> 
> I can change the size of the default buffer if required.
> 
> 
> Diffs
> -----
> 
>   backends/virtuoso/odbcqueryresult.cpp a4f2387 
> 
> Diff: http://git.reviewboard.kde.org/r/106467/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vishesh Handa
> 
>