D11753: baloodb: Add clean command

This revision was automatically updated to reflect the committed changes. Closed by commit R293:675d989823ec: baloodb: Add clean command (authored by michaelh). REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32320=32565 REVISION DETAIL

D11753: baloodb: Add clean command

bruns accepted this revision. This revision is now accepted and ready to land. REPOSITORY R293 Baloo BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, cfeck, smithjd, ashaposhnikov,

D11753: baloodb: Add clean command

michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:173 > Pass in a List/Set of deviceIds, and return the same type. Your suggestion in D1 can be reused. I'll do it there. > bruns wrote in databasesanitizer.cpp:187 >

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32320. michaelh marked 17 inline comments as done. michaelh added a comment. - Apply suggested changes REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32255=32320 BRANCH sanitize-clean (branched from master)

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:173 > + > +QMap deviceFilters(QVector& infos, const > DatabaseSanitizer::ItemAccessFilters accessFilter) > +{ Pass in a List/Set of deviceIds, and return the same type. > michaelh wrote in

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:187 > + > +bool isIgnored(const QStorageInfo& storageInfo, const > DatabaseSanitizer::ItemAccessFilters accessFilter) > +{ Are you planning to use this one anywhere else? Otherwise, inside `deviceFilters(...)`:

D11753: baloodb: Add clean command

michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:344 > do the calls to m_transaction using a wrapper, e.g. > `DatabaseSanitizer::removeDocument(info.id)` I'm assuming you meant `DatabaseSanitizerImpl::removeDocument(info.id)`. If you meant

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32255. michaelh added a comment. - Wrap transaction operations REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32254=32255 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32254. michaelh added a comment. - Remove unrelated whitespace change REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32201=32254 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:344 > +} else { > +m_pimpl->m_transaction->removeDocument(info.id); > +out << i18n("Removing:"); do the calls to m_transaction using a wrapper, e.g.

D11753: baloodb: Add clean command

michaelh removed a dependent revision: D12044: baloodb: Improve interface. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32201. michaelh added a comment. - Rebase REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32175=32201 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.kde.org/D11753

D11753: baloodb: Add clean command

michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:166 > Why do you remove the private: here? 340: m_pimpl->m_transaction->removeDocument(info.id); and more > bruns wrote in databasesanitizer.cpp:343 > remove `sep`, use `QStringLiteral(" device: %1") instead

D11753: baloodb: Add clean command

michaelh added a dependent revision: D1: baloodb: Use complete access filtering for all outputs. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks, bruns Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun,

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32175. michaelh added a comment. - Improve access filtering REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=32105=32175 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

bruns requested changes to this revision. bruns added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > databasesanitizer.cpp:166 > > -private: > Transaction* m_transaction; Why do you remove the private: here? > databasesanitizer.cpp:176 > +

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 32105. michaelh added a comment. - Adapt to upstream changes REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=31208=32105 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

michaelh added a task: T8250: Sanitize the database. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh planned changes to this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh added a dependent revision: D12044: baloodb: Improve interface. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: bruns, cfeck, smithjd, ashaposhnikov, michaelh, astippich, spoorun, ngraham, alexeymin

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > michaelh wrote in databasesanitizer.cpp:333 > 8 meanings of MOOD acronym or abbreviation. > MOOD stands for > > Magic of Ordinary Days > Meet only Original Designs > Michigan Out of Doors > Movie Organizer Online Database > Miata Owners

D11753: baloodb: Add clean command

michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:333 > Fair reason, the comment below is also mood then ... 8 meanings of MOOD acronym or abbreviation. MOOD stands for Magic of Ordinary Days Meet only Original Designs Michigan Out of Doors Movie

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > michaelh wrote in databasesanitizer.cpp:333 > I did that before, but decided on `--dry-run` to simulate the process as > close as possible. > Also, compared to the time it takes to collect the data, removing the docs is > completely neglectible.

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 31208. michaelh added a comment. - Ignore symbolic links There seems to be no reliable way to map a symbolic link's target to a device id. Instead of removing it print a parseable message so users can remove it manually. REPOSITORY R293 Baloo

D11753: baloodb: Add clean command

michaelh planned changes to this revision. michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:316 > What do you consider a "truly dead" symlink? > > If you really want to do it correctly, you have to walk the file system > yourself, one symlink target path

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > michaelh wrote in databasesanitizer.cpp:316 > I've tried it. Sadly your suggestion does not work. With `fi = > filePathToStat(info.symlink.toLocal8Bit())` fi._st_dev is == 0 when the > symlink target does not exist. Hence it does tell me **why**

D11753: baloodb: Add clean command

michaelh marked an inline comment as done. michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:316 > I think it is better to use > QT_FSTAT(info.symlink.toLocal8Bit().constData(), ...) here, avoids lots of > calls to the database, and guarantees more

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 31033. michaelh added a comment. - Apply suggested change REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=30737=31033 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

michaelh added inline comments. INLINE COMMENTS > bruns wrote in databasesanitizer.cpp:333 > why not just make the removeDocument transaction above depend on dryRun? > removeDocument is expensive ... I did that before, but decided on `--dry-run` to simulate the process as close as possible.

D11753: baloodb: Add clean command

bruns added inline comments. INLINE COMMENTS > databasesanitizer.cpp:113 > info.url = url; > -info.accessible = !url.isEmpty() && QFileInfo::exists(url); > +info.accessible = !url.isEmpty() && fileInfo.exists(url); > +info.symlink =

D11753: baloodb: Add clean command

michaelh updated this revision to Diff 30737. michaelh added a comment. - Account for symbolic links REPOSITORY R293 Baloo CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D11753?vs=30726=30737 BRANCH sanitize-clean (branched from master) REVISION DETAIL

D11753: baloodb: Add clean command

michaelh edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: smithjd, ashaposhnikov, michaelh, astippich, spoorun, nicolasfella, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh edited the summary of this revision. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: smithjd, ashaposhnikov, michaelh, astippich, spoorun, nicolasfella, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh added a dependency: D11745: databasesanitizer: Use flags for filtering. REPOSITORY R293 Baloo REVISION DETAIL https://phabricator.kde.org/D11753 To: michaelh, #baloo, #frameworks Cc: smithjd, ashaposhnikov, michaelh, astippich, spoorun, nicolasfella, ngraham, alexeymin

D11753: baloodb: Add clean command

michaelh created this revision. michaelh added reviewers: Baloo, Frameworks. Restricted Application added projects: Frameworks, Baloo. michaelh requested review of this revision. REVISION SUMMARY Remove stale entries from database This is mostly based on D11529