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&id=32565 REVISION DETAIL https://pha

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, michaelh,

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 > A

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&id=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 databasesanitizer.cpp:343 >

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(...)`: aut

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 `DatabaseSanitiz

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&id=32255 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator

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&id=32254 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phab

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. `DatabaseSanitizer::removeDocumen

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&id=32201 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.kde.org/D11753 AFFE

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, ngr

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&id=32175 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.kd

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 > +quint6

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&id=32105 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.k

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 on

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 Org

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. C

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 co

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** th

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 consistent

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&id=31033 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.kde.

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. A

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 = fileInfo.isSymL

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&id=30737 BRANCH sanitize-clean (branched from master) REVISION DETAIL https://phabricator.

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