D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

bruns added a comment.


  In D28400#643965 , @bcooksley 
wrote:
  
  > Sorry, but that isn't how this works. Also, you will notice that one of the 
failing platforms is FreeBSD. Which is freely available and OSS.
  
  
  That does not change the fact it requires extra effort to install it and keep 
it up to date.
  
  > The responsibility of people to keep code compiling rests with those 
working on it. Should there be platform specific issues they may from time to 
time get assistance from those who look after those platforms, but in general 
the responsibility lies with the person working on the code.
  
  Thats your opinion. Mine differs.
  
  > The FreeBSD failure is most likely due to Clang being more strict with C++ 
than GCC is.
  
  "most likely". So maybe, maybe not...
  
  > As for Windows, that looks to be a namespacing issue.
  
  More guesswork ...

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: vkrause, bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Volker Krause]

vkrause added inline comments.

INLINE COMMENTS

> bruns wrote in searchstore.cpp:82
> Thanks for the heads-up.
> 
> As you have noticed, the message is vague, so someone with access to one of 
> the affected systems should test it and submit a review.

I don't have either of those here to test it for sure, but I have fixed similar 
errors elsewhere recently for those platforms by just adding the  
include, so I think @kossebau is right.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: vkrause, bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Ben Cooksley]

bcooksley added a comment.


  Sorry, but that isn't how this works. Also, you will notice that one of the 
failing platforms is FreeBSD. Which is freely available and OSS.
  
  The responsibility of people to keep code compiling rests with those working 
on it. Should there be platform specific issues they may from time to time get 
assistance from those who look after those platforms, but in general the 
responsibility lies with the person working on the code.
  
  The FreeBSD failure is most likely due to Clang being more strict with C++ 
than GCC is.
  
  As for Windows, that looks to be a namespacing issue.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> kossebau wrote in searchstore.cpp:82
> This might need an explicit `#include ` on some systems, not exactly 
> sure by the error message.
> 
> At least with FreeBSD & Windows KDE CI fails over this: 
> https://build.kde.org/view/Failing/job/Frameworks/job/baloo/

Thanks for the heads-up.

As you have noticed, the message is vague, so someone with access to one of the 
affected systems should test it and submit a review.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

bruns added a comment.


  In D28400#643934 , @bcooksley 
wrote:
  
  > The following is notice that the following reviews/commits are being 
scheduled to be reverted in 24 hours due to the FTBFS on Windows and FreeBSD:
  >
  > - 2b9c468816459a318dd2c8fe96e5e5acf1cedfd1 

  > - 2fa16a2865dc385f1106c3eadb363bbe9d1244b1 

  > - d4494ee496640325fb6fd72b80f12c5c4b124d22 

  
  
  This is complete nonsense. It is the responsibility of the #windows 
 and #freebsd 
 teams to keep their builds working. 
If Windows/FreeBSD keep failing, *their builds* should be scheduled for removal.
  
  I have no WIndows license, I have no Visual Studio license, I don't care for 
WIndows, and I won't invest any time in it.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Ben Cooksley]

bcooksley added a comment.


  The following is notice that the following reviews/commits are being 
scheduled to be reverted in 24 hours due to the FTBFS on Windows and FreeBSD:
  
  - 2b9c468816459a318dd2c8fe96e5e5acf1cedfd1 

  - 2fa16a2865dc385f1106c3eadb363bbe9d1244b1 

  - d4494ee496640325fb6fd72b80f12c5c4b124d22 


REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: bcooksley, kossebau, kde-frameworks-devel, hurikhan77, lots0logs, 
LeGast00n, cblack, fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Friedrich W. H. Kossebau]

kossebau added inline comments.

INLINE COMMENTS

> searchstore.cpp:82
> +};
> +constexpr std::array internalProperties {{
> +{ "filename","F",QVariant::String },

This might need an explicit `#include ` on some systems, not exactly 
sure by the error message.

At least with FreeBSD & Windows KDE CI fails over this: 
https://build.kde.org/view/Failing/job/Frameworks/job/baloo/

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: kossebau, kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R293:2b9c46881645: [AdvancedQueryParser] Move semantic 
handling of tokens to SearchStore (authored by bruns).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D28400?vs=78783=79584#toc

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28400?vs=78783=79584

REVISION DETAIL
  https://phabricator.kde.org/D28400

AFFECTED FILES
  autotests/unit/lib/advancedqueryparsertest.cpp
  src/engine/postingdb.cpp
  src/engine/postingdb.h
  src/engine/transaction.cpp
  src/engine/transaction.h
  src/lib/advancedqueryparser.cpp
  src/lib/searchstore.cpp
  src/lib/searchstore.h

To: bruns, #baloo, ngraham
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Nathaniel Graham]

ngraham accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R293 Baloo

BRANCH
  submit

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

bruns added a dependent revision: D28401: [SearchStore] Handle double values in 
property queries.

REPOSITORY
  R293 Baloo

REVISION DETAIL
  https://phabricator.kde.org/D28400

To: bruns, #baloo, ngraham
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D28400: [AdvancedQueryParser] Move semantic handling of tokens to SearchStore

[Stefan Brüns]

bruns created this revision.
bruns added reviewers: Baloo, ngraham.
Herald added projects: Frameworks, Baloo.
Herald added a subscriber: kde-frameworks-devel.
bruns requested review of this revision.

REVISION SUMMARY
  The AdvancedQueryParser has no knowledge about the semantics of a
  value, while the searchstore can use KFM::PropertyInfo to determine
  if the value should be converted to e.g. an integer or a QDateTime.
  
  This allows to e.g. use CreationTime in queries.

TEST PLAN
  baloosearch 'creationtime>=2010-01-20'
  baloosearch 'creationDate>="1997-01-01 10:00:00"' \
  
AND 'creationDate<=2000-01-01'

REPOSITORY
  R293 Baloo

BRANCH
  submit

REVISION DETAIL
  https://phabricator.kde.org/D28400

AFFECTED FILES
  autotests/unit/lib/advancedqueryparsertest.cpp
  src/engine/postingdb.cpp
  src/engine/postingdb.h
  src/engine/transaction.cpp
  src/engine/transaction.h
  src/lib/advancedqueryparser.cpp
  src/lib/searchstore.cpp
  src/lib/searchstore.h

To: bruns, #baloo, ngraham
Cc: kde-frameworks-devel, hurikhan77, lots0logs, LeGast00n, cblack, 
fbampaloukas, GB_2, domson, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams