D13425: File indexer closing improvements.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien, #baloo, poboiko
Cc: ngraham, mgallien, kde-frameworks-devel, bruns, #baloo, LeGast00n, 
fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, abrahams

D16579: Remove support for non-standard APE tag field names from the test files

[James Smith]

smithjd added a comment.


  In D16579#428993 , @bruns wrote:
  
  > The change, as you pushed it, was never open for review, so there was not 
even the chance to review.
  
  
  Diff 44708 included these changes for review. No one requested changes then, 
and they were ack'ed since: 
https://phabricator.kde.org/D16579?vs=on=44708=ignore-most#420650 
.
  
  > Hello, the tone of your comment is not appropriate.
  
  You might as well have said, 'the change as you pushed it, fulfilled my 
request for changes, and there was not even a chance to lift it'. That's now 
praise instead of misinformation.
  
  > from the latest comments, it's not clear that their concerns were addressed 
first.
  
  Also misinformation: 
https://phabricator.kde.org/D16579?vs=on=44708=ignore-most#420650.
 There was also no dissension during the time since this comment was posted and 
the review was closed.
  
  If any subset of review changes were already in principle agreed upon and 
with no requested changes, //AND// the code requested changed is to be removed 
from the final patch, //AND// the changes are binary-only, and the differences 
not rendered on the review ticket:
  
  A) Further review is unnecessary, if only to allow the reviewers to cancel 
their (obsolete) change requests before it is closed;
  B) I'm not aware where it is required that ALL reviewers must cancel their 
requested changes before the review is closed. The review could have been just 
as simply closed without the commit hook and the agreed repository changes made 
without referencing the review in the commit message, of course ignoring the 
fabulous background this review brought before it was turned into an argument 
over why it was closed with 2 reviewers still left to cancel their 
(superfluous) involvement as they see fit.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: ngraham, bruns, astippich, kde-frameworks-devel, #baloo, gennad, domson, 
ashaposhnikov, michaelh, spoorun, abrahams

D16579: Remove support for non-standard APE tag field names from the test files

[James Smith]

smithjd added a comment.


  The change was ack'ed in this review, and this review closed by the commit.
  
  There was no code that required further review, only binary changes. The 
posted concerns were addressed by D18826 .
  
  If you really are that concerned that this review is cleaned up, request 
yourself that the reviewers that requested changes clear their request. I don't 
need to ask them to when the request is no longer valid. Otherwise, please at 
least read the full review closely before complaining about events you weren't 
consulted on or involved with, and have imo little reason to force a re-visit 
of.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: ngraham, bruns, astippich, kde-frameworks-devel, #baloo, gennad, domson, 
ashaposhnikov, michaelh, spoorun, abrahams

D16579: Remove support for non-standard APE tag field names from the test files

[James Smith]

smithjd added a comment.


  In D16579#428790 , @ngraham wrote:
  
  > Two reviewers had open "Changes Requested" statuses on this patch when you 
committed it, and from the latest comments, it's not clear that their concerns 
were addressed first. Can you explain your thought process regarding why you 
felt it was appropriate to ignore your reviewers as well as KDE community norms 
and commit this anyway?
  
  
  The scope has shrunk to just the test files since D18826 
 made the original code changes and the 
requested changes obsolete, and the test file changes were ack'ed.
  
  Your comment is unfounded.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: ngraham, bruns, astippich, kde-frameworks-devel, #baloo, gennad, domson, 
ashaposhnikov, michaelh, spoorun, abrahams

D16579: Remove support for non-standard APE tag field names from the test files

[James Smith]

smithjd retitled this revision from "Remove support for non-standard APE tag 
field names" to "Remove support for non-standard APE tag field names from the 
test files".
smithjd edited the summary of this revision.
smithjd edited the test plan for this revision.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, gennad, domson, 
ashaposhnikov, michaelh, spoorun, ngraham, abrahams

D16579: Remove support for non-standard APE tag field names

[James Smith]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R286:4c5c4f5b91ab: Use ALBUM ARTIST field name for 
APEv2 test files. (authored by smithjd).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D16579?vs=52185=53610#toc

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16579?vs=52185=53610

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

AFFECTED FILES
  autotests/samplefiles/test.ape
  autotests/samplefiles/test.mpc

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, gennad, domson, 
ashaposhnikov, michaelh, spoorun, ngraham, abrahams

D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

[James Smith]

smithjd added a comment.


  This test fails:
  
  FAIL!  : TagLibExtractorTest::testMp4(mp4) Compared values are not the same
  
Actual   (resultMp4.properties().value(Property::Rating).toInt()): 0
Expected (8) : 8
Loc: 
[/home/enderw/Code/kfilemetadata/autotests/taglibextractortest.cpp(381)]
  
  Is this a new failure?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, ngraham, bruns, mgallien
Cc: smithjd, kde-frameworks-devel, #baloo, gennad, domson, ashaposhnikov, 
michaelh, astippich, spoorun, ngraham, bruns, abrahams

D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

[James Smith]

smithjd added inline comments.

INLINE COMMENTS

> astippich wrote in taglibextractor.cpp:195
> I wanted to do so first, but that will require to also put the PropertyMap 
> into the extractAsfTags method, which I think is not worth it.

lstASF = asfTags->attribute("WM/Writer");
...

The existing code does this.

REPOSITORY
  R286 KFileMetaData

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

To: astippich, ngraham, bruns, mgallien
Cc: smithjd, kde-frameworks-devel, #baloo, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D18826: Rewrite the taglib extractor to use the generic PropertyMap interface

[James Smith]

smithjd added inline comments.

INLINE COMMENTS

> taglibextractor.cpp:109
> +{
> +if (savedProperties.contains("TITLE")) {
> +result->add(Property::Title, 
> TStringToQString(savedProperties["TITLE"].toString()).trimmed());

This could return early if the property map is empty.

> taglibextractor.cpp:195
>  }
> -
> -itApe = lstApe.find("PERFORMER");
> -if (itApe != lstApe.end()) {
> -if (!data.performer.isEmpty()) {
> -data.performer += ", ";
> +/* Lyricist is called "WRITER" for wma/asf files */
> +if (savedProperties.contains("WRITER")) {

Can this be moved into the asf-specific extractions?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, ngraham, bruns, mgallien
Cc: smithjd, kde-frameworks-devel, #baloo, domson, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D16579: Remove support for non-standard APE tag field names

[James Smith]

smithjd added a comment.


  > Anyway, most of this special handling of mimetypes goes away with D18826 
, where TagLib will do the conversion 
automatically.
  
  The test files should reflect the most common tag fieldnames; 'albumartist' 
is used in the APEv2 test files instead of 'album artist'.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, domson, ashaposhnikov, 
michaelh, spoorun, ngraham, abrahams

D16579: Remove support for non-standard APE tag field names

[James Smith]

smithjd retitled this revision from "Fix APE tag extraction" to "Remove support 
for non-standard APE tag field names".
smithjd edited the summary of this revision.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Fix APE tag extraction

[James Smith]

smithjd added a comment.


  I realise that with all the fields referenced here having at least one 
standard fieldname parsed now that complaining may seem to be flogging a dead 
horse, but I'd like to re-iterate that identically-purposed fields should not 
be encouraged. This is at best a specialized re-purposing that really shouldn't 
be. Maintaining a branch / fork for this is probably appropriate.
  
  Lastly, I'd also like to point out the TagLib documentation, that 
specifically mentions a conversion from 'Album Artist' in 'AlbumArtist' for 
compatibility with other tags: 
https://taglib.org/api/classTagLib_1_1APE_1_1Tag.html#af77a10659fbb0018228420ad6de501e1.
 'Disc' is also converted from APE, but you have to dig into the code to find 
this: https://github.com/taglib/taglib/blob/master/taglib/ape/apetag.cpp#L211

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Fix APE tag extraction

[James Smith]

smithjd updated this revision to Diff 52185.
smithjd added a comment.


  - Use the de-facto Album Artist field name for APE tags.
  
  Re-base.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16579?vs=44708=52185

BRANCH
  master-musepackFixes (branched from master)

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

AFFECTED FILES
  autotests/samplefiles/test.ape
  autotests/samplefiles/test.mpc
  autotests/samplefiles/test.wv
  src/extractors/taglibextractor.cpp

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd added a comment.


  "Locking" the scheduler is simpler here than identifying that it "has gone 
idle". "Locking" only happens here for a small number of runnables (two) that 
can't run concurrently.

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 49977.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.
  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  - Simplify locking the scheduler.
  - Adapt index cleaner to scheduler.
  - Rebase

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=43623=49977

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D16579: Fix APE tag extraction

[James Smith]

smithjd added a comment.


  In D16579#352887 , @astippich 
wrote:
  
  > You're doing the exact opposite of what we're asking for.
  >  Look, I'd love to merge the bug fix for the DISC property. But we need 
compatibility.
  >  I'll give you another reason: KFileMetaData has basically required that 
users use the DISCNUMBER field until now. And now you want to change that 
without providing any suitable transition. That's not user-friendly.
  
  
  Despite the large amount of tagger information attached to this review, it's 
still impossible to find a tagger that defaults to an alternative to the DISC 
field name for APE, Some taggers implement the ability to add arbitrary fields 
to APE tags, and nothing is stopping anyone from populating a DISCNUMBER field, 
though ignoring the more-established DISC field name is counterintuitive and 
this should be regarded as a specialized use-case of the APE tag. Similarly the 
non-standard AlbumArtist alternative to the Album Artist APE field name has 
been accepted (albeit as an alternative to an established field name) since 
about the time the disk number property was introduced. Because one of the 
primary motivations behind multimedia tagging is seemless portability, 
specialized application of APE tags should not be encouraged over existing 
standard field names.
  
  Forcing compliance with established tag field names is lower maintenance, and 
not the end of the world for KDE APE tag users. Forcing these users to adopt 
established field names to make full use of the KDE metadata infrastructure is 
a responsible thing to do anyway, and users already understand they must put 
effort into their tags to reap the full benefit anyway. Ending the parsing of 
field names that have widely-implemented counterparts should be effected with 
no regard for compatibility with *any* specialised application of APE tags, and 
with *no* transition period. KDE is very specific about the types of metadata 
it can make use of and requiring users to make the extra effort to get better 
use of its capabilities is not unreasonable. It is also not unreasonable for 
the user to expect that this effort would be maximally re-useable elsewhere by 
default.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Fix APE tag extraction

[James Smith]

smithjd added a comment.


  In D16579#352788 , @smithjd wrote:
  
  > - Use the de-facto Album Artist field name for APE tags.
  >
  >   Since the discussion around this patch has also included the Album Artist 
field, add the changes required for this field to this review.
  
  
  Also adding that EasyTag uses the 'Album Artist' field name. Not sure about 
picard, but the above-linked documentation seems to suggest this.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Fix APE tag extraction

[James Smith]

smithjd retitled this revision from "Musepack disk number field name is DISC." 
to "Fix APE tag extraction".
smithjd edited the summary of this revision.
smithjd edited the test plan for this revision.
smithjd added a reviewer: mgallien.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns, mgallien
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd updated this revision to Diff 44708.
smithjd added a comment.


  - Use the de-facto Album Artist field name for APE tags.
  
  Since the discussion around this patch has also included the Album Artist
  field, add the changes required for this field to this review.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16579?vs=44638=44708

BRANCH
  master-musepackFixes (branched from master)

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

AFFECTED FILES
  autotests/samplefiles/test.ape
  autotests/samplefiles/test.mpc
  autotests/samplefiles/test.wv
  autotests/taglibextractortest.cpp
  src/extractors/taglibextractor.cpp

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#352282 , @bruns wrote:
  
  > In D16579#352279 , @smithjd 
wrote:
  >
  > > I would instead recommend a tag editor that properly tags APE files, such 
as puddletag. APE-using formats are less mainstream than id3 using formats. 
Users with APE-using formats usually know WHY they use their format of choice, 
and will know there are potential support shortfalls such as non-compliant 
tagging software, or software that allows to circumvent the accepted tagging 
standard, and will know how to mitigate or avoid such issues. I must also point 
out that stricter tag field adherence is better for APE-using formats in 
particular, and is better for all tag-using formats in general.
  >
  >
  > Good luck "educating" every APE user, they will really like to be told they 
should do what you consider right.
  
  
  ...and we should probably rip out *EVERY* other format except for mp3 with 
its larger installed base and because it's standard fields are more supported 
by other software.
  
  This is a *fix* for APE tag support, not an argument over why there are 
*de-facto* standard tag fields or why certain users can screw up with powerful 
software, or why standardization is a problem and goal, especially in a tag 
format that permits arbitrary field names. There are 2 sources attached to this 
review and anecdotal evidence as to what a number of tag programs can or do 
write into APE tags. This is enough for an educated guess as to a broad 
consensus on standardized APE field names.
  
  I'm also not in favour of compatibility with alternatives to the de-facto APE 
field names. We should not be encouraging alternatives to well-established, 
documented field names. It's also potential burden on the writers of other 
software and also on the users of APE tags, and detrimental to the APE-using 
format's ecosystem.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  I would instead recommend a tag editor that properly tags APE files, such as 
puddletag. APE-using formats are less mainstream than id3 using formats. Users 
with APE-using formats usually know WHY they use their format of choice, and 
will know there are potential support shortfalls such as non-compliant tagging 
software, or software that allows to circumvent the accepted tagging standard, 
and will know how to mitigate or avoid such issues. I must also point out that 
stricter tag field adherence is better for APE-using formats in particular, and 
is better for all tag-using formats in general.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  I would also argue that accepting values from tag field names that have 
identically-purposed, widely-acceptable alternatives is irresponsible. Changing 
your tags to meet the standard then is a more viable course of action.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich, bruns
Cc: bruns, astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
spoorun, ngraham, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#352191 , @astippich 
wrote:
  
  > In D16579#352147 , @smithjd 
wrote:
  >
  > > In D16579#351910 , @astippich 
wrote:
  > >
  > > > The ape tag tests fail with this patch, but the test is actually wrong 
in that regard. It tests for an empty disc number, which I haven't noticed 
before.
  > > >  I've found references to both DISCNUMBER and DISC, so the safest way 
is probably to check both.
  > > >  So please query both tags like it is already done for the album artist 
and adjust the taglibextractortest.
  > >
  > >
  > > DISCNUMBER doesn't seem to be a valid (or widely used) field for APEv2.
  > >
  > > More (Picard) information: https://picard.musicbrainz.org/docs/mappings
  > >
  > > Also it appears Picard, Puddletag and ffmpeg use 'Album Artist', not 
'ALBUMARTIST'. And the link I provided 
(https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping) also lists 'Album 
Artist' not 'ALBUMARTIST'. The unit test file has an 'ALBUMARTIST' field that 
should be changed to 'Album Artist'.
  >
  >
  > Yes, maybe it's not widely used, but it is used. Kodi for example supports 
both reading from DISCNUMBER and DISC. If you use kid3 to edit the metadata of 
ape tags, the standard behavior is to actually to write to DISCNUMBER (and 
similar to ALBUMARTIST).
  >  One thing I've learned when I digged into metadata of audio files is that 
there is no standard, and KFileMetaData should handle as much cases as 
possible. Since it is easy to query both, please add it.
  
  
  kid3 allows arbitrary field names and the APE tag field names for DISC and 
ALBUM ARTIST aren't there by default. The corresponding tags for id3 are there 
and can be applied to APE tags, but they aren't widely used as valid APE fields.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd updated this revision to Diff 44638.
smithjd added a comment.


  - Change the unit test to check if the APE 'disc' value is correct.

REPOSITORY
  R286 KFileMetaData

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16579?vs=44607=44638

BRANCH
  master-musepackFixes (branched from master)

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

AFFECTED FILES
  autotests/taglibextractortest.cpp
  src/extractors/taglibextractor.cpp

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  In D16579#351910 , @astippich 
wrote:
  
  > The ape tag tests fail with this patch, but the test is actually wrong in 
that regard. It tests for an empty disc number, which I haven't noticed before.
  >  I've found references to both DISCNUMBER and DISC, so the safest way is 
probably to check both.
  >  So please query both tags like it is already done for the album artist and 
adjust the taglibextractortest.
  
  
  DISCNUMBER doesn't seem to be a valid (or widely used) field for APEv2.
  
  More (Picard) information: https://picard.musicbrainz.org/docs/mappings
  
  Also it appears Picard, Puddletag and ffmpeg use 'Album Artist', not 
'ALBUMARTIST'. And the link I provided 
(https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping) also lists 'Album 
Artist' not 'ALBUMARTIST'. The unit test file has an 'ALBUMARTIST' field that 
should be changed to 'Album Artist'.

REPOSITORY
  R286 KFileMetaData

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

To: smithjd, astippich
Cc: astippich, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd added a comment.


  Useful tag mapping information:
  
  https://wiki.hydrogenaud.io/index.php?title=Tag_Mapping

REPOSITORY
  R286 KFileMetaData

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

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D16579: Musepack disk number field name is DISC.

[James Smith]

smithjd created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
smithjd requested review of this revision.

REVISION SUMMARY
  Fix the Musepack disk field.

REPOSITORY
  R286 KFileMetaData

BRANCH
  master-musepackFixes (branched from master)

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

AFFECTED FILES
  src/extractors/taglibextractor.cpp

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 43623.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.
  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  - Simplify locking the scheduler.
  - Adapt index cleaner to scheduler.
  
  Exporting the storage object is not part of this patchset.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=43355=43623

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D15939: Perform checks for unindexed files and stale index entries on startup

[James Smith]

smithjd added a comment.


  In D15939#341728 , @poboiko wrote:
  
  > In D15939#341094 , @smithjd 
wrote:
  >
  > > https://phabricator.kde.org/D11529 was already up for review, implemented 
the index cleaner and checked for removeable volumes before removing index 
entries. Exporting the storagedevices object was required: 
https://phabricator.kde.org/D15047.  The alternative, implementing a path 
lookup function is here: https://phabricator.kde.org/D15843.
  >
  >
  > Sorry, I was not aware of those patches (didn't do my homework). I'll look 
at those!
  >
  > UPD: but will those help? I mean, if device is not present at the moment, 
`Solid` won't pop it up, and index entries will be forgotten. 
  >  It seems like it will help only if device is present, but not yet mounted.
  
  
  Yes. The volume is ignored even if the storage device is unplugged from boot.

REPOSITORY
  R293 Baloo

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

To: poboiko, #baloo, #frameworks, vhanda, ngraham
Cc: smithjd, bruns, ngraham, kde-frameworks-devel, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D15939: Perform checks for unindexed files and stale index entries on startup

[James Smith]

smithjd added a comment.


  https://phabricator.kde.org/D11529 was already up for review, implemented the 
index cleaner and checked for removeable volumes before removing index entries. 
Exporting the storagedevices object was required: 
https://phabricator.kde.org/D15047.  The alternative, implementing a path 
lookup function is here: https://phabricator.kde.org/D15843.

REPOSITORY
  R293 Baloo

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

To: poboiko, #baloo, #frameworks, vhanda, ngraham
Cc: smithjd, bruns, ngraham, kde-frameworks-devel, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd updated this revision to Diff 43368.
smithjd added a comment.


  Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=43363=43368

BRANCH
  master-scheduler-haltonquit (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/mainhub.cpp

To: smithjd, bruns, mgallien, #baloo, poboiko
Cc: ngraham, mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, 
michaelh, astippich, spoorun, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd added a comment.


  https://bugs.kde.org/show_bug.cgi?id=353559

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien, #baloo, poboiko
Cc: ngraham, mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, 
michaelh, astippich, spoorun, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd removed a dependency: D13216: Overhaul the file index scheduler..

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik, bruns
Cc: bruns, kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd removed a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd removed a dependency: D15047: Export the StorageDevices object..

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik, bruns
Cc: bruns, kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd removed a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, broulik, #baloo, bruns, poboiko
Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd updated this revision to Diff 43363.
smithjd added a comment.


  Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=43361=43363

BRANCH
  master-scheduler-watchesInstalled (branched from master)

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

AFFECTED FILES
  src/file/mainhub.cpp

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd updated this revision to Diff 43361.
smithjd added a comment.


  Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=40344=43361

BRANCH
  master-scheduler-haltonquit (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/mainhub.cpp

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd updated this revision to Diff 43357.
smithjd added a comment.


  - Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15047?vs=43353=43357

BRANCH
  master-storageDevicesExport (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h

To: smithjd, broulik, #baloo, bruns, poboiko
Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 43355.
smithjd marked an inline comment as done.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.
  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  - Simplify locking the scheduler.
  - Adapt index cleaner to scheduler.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=40393=43355

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd updated this revision to Diff 43353.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.
  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  - Simplify locking the scheduler.
  - Adapt index cleaner to scheduler.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15047?vs=40341=43353

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  CMakeLists.txt
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, broulik
Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D15939: Perform checks for unindexed files and stale index entries on startup

[James Smith]

smithjd added a comment.


  Does this run at startup? If so, this will erase the entries of files on a 
removable volume not already mounted.

REPOSITORY
  R293 Baloo

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

To: poboiko, #baloo, #frameworks, vhanda, ngraham
Cc: smithjd, bruns, ngraham, kde-frameworks-devel, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik, bruns
Cc: bruns, kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd added a comment.


  In D15843#334518 , @ngraham wrote:
  
  > Perhaps we should discuss the implementation of multi-device indexing in a 
Phab ticket instead of across the comments of multiple patches. Then we can 
settle on an agreed-upon approach and go implement it instead of going in 
circles with different approaches.
  
  
  This patch (or the other) is a requirement to augment the **current 
implementation** indexing of multiple devices with **safe** cleanup of the dead 
entries of multiple devices. It does not care about potential problems with the 
generated document id used by the database.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, bruns
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd added a comment.


  >>> As a side note - cross-device indexing is broken anyway and can never 
work reliably in the current scheme using device ids as part of the baloo 
document id. Device ids are not stable. For the home partition it works 
somewhat reliable as the device setup is sufficiently fixed, but for any 
network share, Vault, disk image, whatever, it depends on the mounting order.
  >> 
  >> 
  >> 
  >>> I will keep opposing any changes trying to paper over it.
  >> 
  >> I have not had a problem with mounted external volumes. FYI, complain to 
the Solid people about the non-randomness of the udi. That isn't a fault of 
Baloo.
  > 
  > You clearly lack understanding of the problem. This is not about solid 
UDIs, but about device ids provided by the kernel (man 7 inode) . Complain to 
the Linux folks!?
  
  That isn't a problem I'm particularly concerned with, that observance being 
unrelated to this patch. Is there a bug filed?

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, bruns
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd added a comment.


  >> There is no need for this, just create a new StorageDevices where you need 
it.
  >> Creating a second StorageDevices instance in a process is quite cheap.
  > 
  > Creating a separate object cluttered the console with duplicated debug 
output and raised a threading error.
  
  No go.
  
  This patch is a little bit tidier than exporting the StorageDevices object. 
Both are sufficient for being able to verify that the path is mounted.
  
  > As a side note - cross-device indexing is broken anyway and can never work 
reliably in the current scheme using device ids as part of the baloo document 
id. Device ids are not stable. For the home partition it works somewhat 
reliable as the device setup is sufficiently fixed, but for any network share, 
Vault, disk image, whatever, it depends on the mounting order.
  
  
  
  > I will keep opposing any changes trying to paper over it.
  
  I have not had a problem with mounted external volumes. FYI, complain to the 
Solid people about the non-randomness of the udi. That isn't a fault of Baloo.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, bruns
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd added a comment.


  In D15843#334230 , @bruns wrote:
  
  > Most obvious problem with this change - as far as I can deduce from your 
description, this is about runtime behaviour. The config class is the wrong 
place to add this method.
  
  
  This isn't that different from shouldBeIndexed and it's related methods. 
Anywhere else would require exporting the StorageDevices object, already 
actively opposed by you: https://phabricator.kde.org/D15047?
  
  Please provide a credible alternative if you're going to oppose new 
additions, or oppose new features and clearly explain your opposition if you 
intend to block them by opposing new additions.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, bruns
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd added a comment.


  In D15843#334024 , @bruns wrote:
  
  > Whats the use case exactly? "Some parts" is not sufficient ...
  
  
  The index cleaner for example needs to skip unmounted paths.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, bruns
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15843: Allow FileIndexerConfig to check device mounted status by path.

[James Smith]

smithjd created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
smithjd requested review of this revision.

REVISION SUMMARY
  Parts of Baloo need some way to check if a path's volume is mounted or not.

REPOSITORY
  R293 Baloo

BRANCH
  master-pathDeviceMounted (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  xattrs are no good for vaults because the xattr is ignored for the 
mountpoint, only restored when the volume is unmounted.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  > No, because even if the default umask is 002, or 022, POSIX says that the 
default permissions of a file on creation are going to be 0666, not 0777:
  
  Trial-and-error proven. :(
  
  In D15718#331449 , @ngraham wrote:
  
  > Let's back up a bit. What's the goal here? What are you trying to 
accomplish?
  
  
  Baloo currently has no method for do-not-index. Perhaps a better way exists 
that prevents new downloads from automatically being indexed and can also 
prevent Baloo vaults from being indexed?
  
  Tracker uses a .trackerignore file. This (or something similar) probably has 
little chance as a default distro safety feature and its usefulness is 
questionable (browsers let you save outside the downloads folder), so this is 
out for the first requirement. This may be a candidate for the second 
requirement.
  
  While this method is already proven, the xdg method of setting tags can fit 
well the second requirement. KDE is sadly lacking in xattr support (no 
'Attributes' tab in file property dialog, xattrs can be silently dropped by KIO 
when moved across filesystems, ...). A variation of the xdg recommendation, 
only recognizing false tags, would also work well.
  
  The first requirement could be solved by ignoring files with no execute bit 
set, though this is opposed. (extra work, etc)

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  https://www.freedesktop.org/wiki/CommonExtendedAttributes suggests xattrs to 
use to store do-not-index information and also requires each file to be 
explicitly set to be indexed.
  
  > user.xdg.robots.index: On a file: "true" to index, "false" to not index. On 
a directory: "true" to traverse the into the directory for indexing, "false" 
for not traversing into it.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo, ltoscano
Cc: ltoscano, marten, bruns, ngraham, kde-frameworks-devel, #baloo, 
ashaposhnikov, michaelh, astippich, spoorun, abrahams

D15583: [Balooctl] remove directory parent check

[James Smith]

smithjd added a comment.


  What was wrong with https://phabricator.kde.org/D15583?id=42018 ?
  
  > I could see the case for allowing this behavior to be explicitly overridden 
by an advanced user who marks ~/foo/ as excluded and then later marks ~foo/bar/ 
as included, but that would be material for another patch I think.
  
  This is actually already how this works, the excludes and includes are both 
resolved into either an index path or do not index path outcome.

REPOSITORY
  R293 Baloo

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

To: ngraham, #baloo, #dolphin, bruns, #frameworks
Cc: smithjd, anthonyfieroni, kde-frameworks-devel, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  In D15718#330864 , @ngraham wrote:
  
  > Making files executable that don't need to be executable is a bad security 
habit. What if the contents get replaced with something malicious? Suddenly 
that now-malicious file has execute permissions.
  
  
  Replacing a file's contents requires write permissions on the file. I have 
plenty of executable shell scripts that aren't an immediate security risk, 
though I suppose if someone gained write privileges over my home statistically 
speaking a shell script is (currently) the most likely choice to gut and 
replace with malicious code. If an attacker already has write permission over 
your home you have bigger problems than a forgotten set-executable file in it 
somewhere anyway. The patched state of that machine's software packages 
dictates how devastating that payload was to your administrator, meanwhile your 
home has probably been wiped.
  
  > 
  > 
  >  ---
  > 
  > Conceptually, you are proposing that the rest of the world adapt to our 
software, rather than the other way around. That's simply not practical. Even 
if this were a good idea, the world will never adapt to us. We must adapt to 
the world. Our software does not exist in a perfect state of total control over 
the environment it inhabits; it exists to facilitate busy people with messy 
lives as they work to accomplish their tasks with a minimum of hassle. That 
goal is not enhanced by breaking KDE Plasma's search tool for them unless they 
give all of their files execute permissions.
  > 
  > Sorry, no go. We need to find a better way.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo
Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  In D15718#330845 , @ngraham wrote:
  
  > In D15718#330844 , @smithjd 
wrote:
  >
  > > In D15718#330836 , @ngraham 
wrote:
  > >
  > > > Wouldn't this have the effect of un-indexing most files? A quick check 
of my documents (text, word processing, excel, etc) reveals that none of them 
have the execute bit set. As-is, I think this would render Baloo mostly useless.
  > >
  > >
  > > A default mask of 0002 or more permissive looks fairly common across 
distros, and is permissive enough to index files by default.
  > >
  > > setfacl -d -m u::rwx ~ or umask 0022 will set default execute permissions 
on created files. You can set something less permissive on your downloads 
directory or plasma vault mount with setfacl -d u::rw or similar.
  > >
  > > chmod -R 755 ~ will recursively give every file in your home directory 
execute permisions.
  >
  >
  > You are proposing fundamentally breaking Baloo and then requiring that 
users or distros clean up our mess for us by making all their files 
executable--in the process reducing their own security. Sorry, but this is 
simply unacceptable.
  >
  > No part of this proposal makes any sense to me. We just can't do this.
  
  
  Folders with the execute bit off already won't allow their contents listed. 
How wouldn't this convention be useful extended to prevent a file from being 
listed in Baloo's database?
  
  How does setting files executable lead to reduced security? Some common 
folders may end up needing more restrictive defaults (downloads for instance) 
and the problem of potentially indexing sensitive information or feeding an 
extractor a specially crafted file designed to exploit a bug in it will have 
been mediated to some extent.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo
Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  In D15718#330836 , @ngraham wrote:
  
  > Wouldn't this have the effect of un-indexing most files? A quick check of 
my documents (text, word processing, excel, etc) reveals that none of them have 
the execute bit set. As-is, I think this would render Baloo mostly useless.
  
  
  A default mask of 0002 or more permissive looks fairly common across distros, 
and is permissive enough to index files by default.
  
  setfacl -d -m u::rwx ~ or umask 0022 will set default execute permissions on 
created files. You can set something less permissive on your downloads 
directory or plasma vault mount with setfacl -d u::rw or similar.
  
  chmod -R 755 ~ will recursively give every file in your home directory 
execute permisions.

REPOSITORY
  R293 Baloo

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

To: smithjd, ngraham, #baloo
Cc: bruns, ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd added a comment.


  Files/folders should not be automatically indexed if the execute bit is 
unset. Downloads from most if not all popular browsers are not executable by 
default.
  
  Setting a file unexecutable would be used to prevent automatic indexing of 
files and most importantly prevent immediate indexing of newly downloaded files 
and indexing of plasma vaults .
  
  Inspiration:
  
  https://phabricator.kde.org/T8066
  https://phabricator.kde.org/D8532

REPOSITORY
  R293 Baloo

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

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15718: Do not index the path if the path has no execute permissions.

[James Smith]

smithjd created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
smithjd requested review of this revision.

REVISION SUMMARY
  This will prevent automatic indexing of newly downloaded files and can be 
used to 
  prevent indexing of Plasma vaults.

REPOSITORY
  R293 Baloo

BRANCH
  master-donotindex (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/filtereddiriterator.cpp
  src/file/modifiedfileindexer.cpp
  src/file/newfileindexer.cpp
  src/file/unindexedfileiterator.cpp
  src/file/xattrindexer.cpp

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

[James Smith]

smithjd added inline comments.

INLINE COMMENTS

> taglibwritertest.cpp:60
>  
> -QCOMPARE(extractedTitle, QStringLiteral("Title1"));
> -QCOMPARE(extractedArtist, QStringLiteral("Artist1"));
> -QCOMPARE(extractedAlbum, QStringLiteral("Album1"));
> +QCOMPARE(extractedTitle, QString(QStringLiteral("Title1") + 
> stringSuffix));
> +QCOMPARE(extractedArtist, QString(QStringLiteral("Artist1") + 
> stringSuffix));

Is wrapping in a QString necessary?

REPOSITORY
  R286 KFileMetaData

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

To: astippich, mgallien, bruns
Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D15583: [Balooctl] remove directory parent check

[James Smith]

smithjd added a comment.


  Technically, this check isn't actually needed, though it does prevent the 
user from entering a path more than once. At first glance this looks like it 
should work:
  
if (folder.startsWith(path))
  
  This doesn't prevent the user from also re-specifying valid paths.

REPOSITORY
  R293 Baloo

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

To: ngraham, #baloo, #dolphin, bruns, #frameworks
Cc: smithjd, anthonyfieroni, kde-frameworks-devel, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd updated this revision to Diff 40394.
smithjd added a comment.


  - New DBus method updateIndex().
  - Document indexing options.
  - Prevent a scheduler race condition.
  - Rebase for separate statechanged functors.
  
  Re-base. Drops setting a runnable priority.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11529?vs=40346=40394

BRANCH
  master-filedeindexer (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexcleaner.cpp
  src/file/indexerstate.h
  src/file/mainhub.cpp
  src/tools/balooctl/main.cpp

To: smithjd, #baloo, vhanda, michaelh, broulik, bruns
Cc: bruns, kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 40393.
smithjd added a comment.


  - Revert "Re-order and use IndexerState to prioritize the indexer thread 
pool."
  
  Avoid setting a priority on the runnables.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=40343=40393

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd added inline comments.

INLINE COMMENTS

> bruns wrote in fileindexscheduler.cpp:117
> Whats that supposed to do (I have an idea, but thats ugly ...)

This starts a QRunnable at a negative priority (in this case the negative 
integer value for NewFiles) and then calls runnableStarted(NewFiles) with the 
enum of the started runnable.

> bruns wrote in fileindexscheduler.h:92
> Make this a QFlags ...

There is no cast from int to enum anywhere here. There is a cast fron enum to 
int, to a negative integer used to set the QRunnable thread priority.

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd added inline comments.

INLINE COMMENTS

> bruns wrote in indexcleaner.cpp:76
> Undesired detach

This example will also detach with or without a const intermediate.

> ...returning a copy from a method means that the copy is shared and has 
> negative side-effects when used with range-based loops. - 
> https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops

> bruns wrote in indexcleaner.cpp:82
> Undesired detach

Same as above.

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik, bruns
Cc: bruns, kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd added a comment.


  In D15047#314741 , @bruns wrote:
  
  > In D15047#314485 , @smithjd 
wrote:
  >
  > > In D15047#314477 , @bruns 
wrote:
  > >
  > > > There is no need for this, just create a new StorageDevices where you 
need it.
  > > >  Creating a second StorageDevices instance in a process is quite cheap.
  > >
  > >
  > > Creating a separate object cluttered the console with duplicated debug 
output and raised a threading error.
  >
  >
  > Then your code is wrong anyway. This is lazy initialization, and if you can 
not not guarantee it is save to do so, don't do it.
  
  
  Duplicated debug output is also undesireable...

REPOSITORY
  R293 Baloo

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

To: smithjd, broulik
Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd edited the summary of this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd updated this revision to Diff 40346.
smithjd added a comment.


  - New DBus method updateIndex().
  - Document indexing options.
  - Prevent a scheduler race condition.
  - Run de-indexing file check in parallel.
  - Rebase for separate statechanged functors.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11529?vs=36419=40346

BRANCH
  master-filedeindexer (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexcleaner.cpp
  src/file/indexerstate.h
  src/file/mainhub.cpp
  src/tools/balooctl/main.cpp

To: smithjd, #baloo, vhanda, michaelh, broulik
Cc: kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd updated this revision to Diff 40345.
smithjd added a comment.


  Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13424?vs=36703=40345

BRANCH
  master-scheduler-watchesInstalled (branched from master)

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

AFFECTED FILES
  src/file/mainhub.cpp

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd updated this revision to Diff 40344.
smithjd added a comment.


  Rebase.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=36702=40344

BRANCH
  master-scheduler-haltonquit (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexerstate.h
  src/file/mainhub.cpp
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 40343.
smithjd added a comment.


  - Re-write the file index scheduler. Combine content indexer suspend logic.
  - Update the balooctl tool with the changed suspend/resume behaviour.
  - Re-order and use IndexerState to prioritize the indexer thread pool.
  - Newline and tab fixes.
  - Prevent a scheduler race condition.
  - Separate runnableStateChanged into two functors.
  - Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=36701=40343

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexerstate.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D15048: Re-write the file index scheduler. Combine content indexer suspend logic.

[James Smith]

smithjd abandoned this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15048: Re-write the file index scheduler. Combine content indexer suspend logic.

[James Smith]

smithjd created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
smithjd requested review of this revision.

REVISION SUMMARY
  Update the balooctl tool with the changed suspend/resume behaviour.
  
  Re-order and use IndexerState to prioritize the indexer thread pool.
  
  Newline and tab fixes.
  
  Prevent a scheduler race condition.
  
  Separate runnableStateChanged into two functors.
  
  Allow the first run indexer to complete before running any other runnables. 
Prevent the new file and unindexed file runnables from running at the same time.

REPOSITORY
  R293 Baloo

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexerstate.h
  src/tools/balooctl/main.cpp

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd marked an inline comment as done.
smithjd added a comment.


  In D15047#314477 , @bruns wrote:
  
  > There is no need for this, just create a new StorageDevices where you need 
it.
  >  Creating a second StorageDevices instance in a process is quite cheap.
  
  
  Creating a separate object cluttered the console with duplicated debug output 
and raised a threading error.

INLINE COMMENTS

> bruns wrote in fileindexerconfig.cpp:322
> This is bad, you remove the const from the QList<>
> Read up about Qt and detaching ...

According to 
https://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops this 
particular example will detach with or without the const intermediate.

REPOSITORY
  R293 Baloo

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

To: smithjd, broulik
Cc: bruns, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd added a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, broulik
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd edited dependencies, added: D15047: Export the StorageDevices object., 
D13216: Overhaul the file index scheduler.; removed: D13424: Run a file changed 
check when the file watches are updated..

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik
Cc: kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd added a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd removed a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd added a reviewer: broulik.
smithjd added a comment.


  This was included in https://phabricator.kde.org/D11529 and is now broken out.

REPOSITORY
  R293 Baloo

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

To: smithjd, broulik
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D15047: Export the StorageDevices object.

[James Smith]

smithjd created this revision.
Herald added projects: Frameworks, Baloo.
Herald added subscribers: Baloo, kde-frameworks-devel.
smithjd requested review of this revision.

REPOSITORY
  R293 Baloo

BRANCH
  master-storageDevicesExport (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h

To: smithjd
Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, 
ngraham, bruns, abrahams

D13425: File indexer closing improvements.

[James Smith]

smithjd retitled this revision from "file indexer closing inprovements." to 
"File indexer closing improvements.".

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd updated this revision to Diff 36703.
smithjd added a comment.


  Change commit description.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13424?vs=36418=36703

BRANCH
  master-scheduler-watchesInstalled (branched from master)

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

AFFECTED FILES
  src/file/mainhub.cpp

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: file indexer closing inprovements.

[James Smith]

smithjd updated this revision to Diff 36702.
smithjd added a comment.


  Change git commit description.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=36416=36702

BRANCH
  master-scheduler-haltonquit (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/mainhub.cpp

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 36701.
smithjd added a comment.


  re-word commits.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=36417=36701

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd updated this revision to Diff 36419.
smithjd added a comment.


  Rebase

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11529?vs=35204=36419

BRANCH
  master-filedeindexer (branched from master)

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

AFFECTED FILES
  src/file/fileindexerconfig.cpp
  src/file/fileindexerconfig.h
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/indexcleaner.cpp
  src/file/indexerstate.h
  src/file/mainhub.cpp
  src/tools/balooctl/main.cpp

To: smithjd, #baloo, vhanda, michaelh, broulik
Cc: kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd removed a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D11529: Balooctl: Deindex unfound files with check command.

[James Smith]

smithjd edited dependencies, added: D13424: Run a file changed check when the 
file watches are updated.; removed: D13216: Overhaul the file index scheduler..

REPOSITORY
  R293 Baloo

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

To: smithjd, #baloo, vhanda, michaelh, broulik
Cc: kde-frameworks-devel, mgallien, ashaposhnikov, michaelh, astippich, 
spoorun, ngraham, bruns, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd added a dependent revision: D11529: Balooctl: Deindex unfound files 
with check command..

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd updated this revision to Diff 36418.
smithjd added a comment.


  - Remove a fixme.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13424?vs=36415=36418

BRANCH
  master-scheduler-watchesInstalled (branched from master)

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

AFFECTED FILES
  src/file/mainhub.cpp

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd removed a dependent revision: D13424: Run a file changed check when the 
file watches are updated..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd edited dependencies, added: D13425: file indexer closing inprovements.; 
removed: D13216: Overhaul the file index scheduler..

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: file indexer closing inprovements.

[James Smith]

smithjd added a dependent revision: D13424: Run a file changed check when the 
file watches are updated..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: file indexer closing inprovements.

[James Smith]

smithjd removed a dependency: D13424: Run a file changed check when the file 
watches are updated..

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd removed a dependent revision: D13425: file indexer closing 
inprovements..

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13216: Overhaul the file index scheduler.

[James Smith]

smithjd updated this revision to Diff 36417.
smithjd added a comment.


  - Re-write the the file index scheduler.
  - Update the balooctl tool with the changed suspend/resume behaviour.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13216?vs=35810=36417

BRANCH
  master-scheduler (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/tools/balooctl/main.cpp

To: smithjd, bruns, mgallien
Cc: ngraham, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, 
spoorun, bruns, abrahams

D13425: file indexer closing inprovements.

[James Smith]

smithjd updated this revision to Diff 36416.
smithjd added a comment.


  Consolidate file indexer close changes into a single review.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13425?vs=35813=36416

BRANCH
  master-scheduler-haltonquit (branched from master)

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

AFFECTED FILES
  src/file/fileindexscheduler.cpp
  src/file/fileindexscheduler.h
  src/file/mainhub.cpp

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13425: file indexer closing inprovements.

[James Smith]

smithjd retitled this revision from "Quit the file indexer when closing." to 
"file indexer closing inprovements.".
smithjd edited the summary of this revision.

REPOSITORY
  R293 Baloo

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

To: smithjd, bruns, mgallien
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13424: Run a file changed check when the file watches are updated.

[James Smith]

smithjd retitled this revision from "On config change halt the file indexer and 
run a file changed check when the file watches are updated." to "Run a file 
changed check when the file watches are updated.".

REPOSITORY
  R293 Baloo

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

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams

D13424: On config change halt the file indexer and run a file changed check when the file watches are updated.

[James Smith]

smithjd updated this revision to Diff 36415.
smithjd added a comment.


  Run a full check for unindexed files when the watches are installed.

REPOSITORY
  R293 Baloo

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D13424?vs=35812=36415

BRANCH
  master-scheduler-watchesInstalled (branched from master)

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

AFFECTED FILES
  src/file/mainhub.cpp

To: smithjd, mgallien, bruns
Cc: mgallien, kde-frameworks-devel, bruns, #baloo, ashaposhnikov, michaelh, 
astippich, spoorun, ngraham, abrahams