diff --git a/libpkg/data/database.cpp b/libpkg/data/database.cpp index c04727b..ccefcda 100644 --- a/libpkg/data/database.cpp +++ b/libpkg/data/database.cpp @@ -894,6 +894,34 @@ PackageSpec LibPkg::PackageUpdater::findPackageWithID(const std::string &package return m_database.m_storage->packageCache.retrieve(*m_database.m_storage, &m_d->packagesTxn, packageName); } +/*! + * \brief Begins updating the existing \a package with the specified \a packageID. + * \remarks + * - Do not use this function when PackageUpdate has been constructed with clear=true. + * - Call this method before modifying \a package. Then modify the package. Then call endUpdate(). + */ +void PackageUpdater::beginUpdate(StorageID packageID, const std::shared_ptr &package) +{ + m_d->update(packageID, true, package); +} + +/*! + * \brief Ends updating the existing \a package with the specified \a packageID. + * \remarks + * - Do not use this function when PackageUpdate has been constructed with clear=true. + * - Call this method after callsing beginUpdate() and modifying \a package. + */ +void PackageUpdater::endUpdate(StorageID packageID, const std::shared_ptr &package) +{ + const auto &storage = m_database.m_storage; + storage->packageCache.store(*m_database.m_storage, m_d->packagesTxn, packageID, package); + m_d->update(packageID, false, package); +} + +/*! + * \brief Updates the specified \a package. The \a package may or may not exist. + * \remarks If the package exists then provides/deps from the existing package are taken over if appropriate. + */ StorageID PackageUpdater::update(const std::shared_ptr &package) { const auto &storage = m_database.m_storage; diff --git a/libpkg/data/database.h b/libpkg/data/database.h index fa74eb3..6f662ab 100644 --- a/libpkg/data/database.h +++ b/libpkg/data/database.h @@ -120,6 +120,8 @@ struct LIBPKG_EXPORT PackageUpdater { ~PackageUpdater(); PackageSpec findPackageWithID(const std::string &packageName); + void beginUpdate(StorageID packageID, const std::shared_ptr &package); + void endUpdate(StorageID packageID, const std::shared_ptr &package); StorageID update(const std::shared_ptr &package); bool insertFromDatabaseFile(const std::string &databaseFilePath); void commit(); diff --git a/libpkg/data/package.cpp b/libpkg/data/package.cpp index 800aa61..5048277 100644 --- a/libpkg/data/package.cpp +++ b/libpkg/data/package.cpp @@ -367,17 +367,26 @@ void PackageBase::clear() description.clear(); } +/*! + * \brief Returns whether addDepsAndProvidesFromOtherPackage() would take over the information (without force). + */ +bool Package::canDepsAndProvidesFromOtherPackage(const Package &otherPackage) const +{ + return !((otherPackage.origin != PackageOrigin::PackageContents && otherPackage.origin != PackageOrigin::CustomSource) + || version != otherPackage.version + || !(!packageInfo || buildDate.isNull() || (otherPackage.packageInfo && buildDate == otherPackage.buildDate))); +} + +/*! + * \brief Takes over deps/provides from \a otherPackage if appropriate. + * \remarks + * The information is not taken over if \a otherPackage does not match the current instance (only + * version and build date are considered) or if otherPackage has no info from package contents. Use + * the \a force parameter to avoid these checks. + */ bool Package::addDepsAndProvidesFromOtherPackage(const Package &otherPackage, bool force) { - if (&otherPackage == this) { - return false; - } - - // skip if otherPackage does not match the current instance (only version and build date are considered) or if otherPackage has no info from package contents - if (!force - && ((otherPackage.origin != PackageOrigin::PackageContents && otherPackage.origin != PackageOrigin::CustomSource) - || version != otherPackage.version - || !(!packageInfo || buildDate.isNull() || (otherPackage.packageInfo && buildDate == otherPackage.buildDate)))) { + if (&otherPackage == this || (!force && !canDepsAndProvidesFromOtherPackage(otherPackage))) { return false; } diff --git a/libpkg/data/package.h b/libpkg/data/package.h index ecb9a93..fa60bfc 100644 --- a/libpkg/data/package.h +++ b/libpkg/data/package.h @@ -415,6 +415,7 @@ struct LIBPKG_EXPORT Package : public PackageBase, std::string_view directoryPath, const ArchiveFile &file, std::set &dllsReferencedByImportLibs); void addDepsAndProvidesFromContents(const FileMap &contents); std::vector processDllsReferencedByImportLibs(std::set &&dllsReferencedByImportLibs); + bool canDepsAndProvidesFromOtherPackage(const Package &otherPackage) const; bool addDepsAndProvidesFromOtherPackage(const Package &otherPackage, bool force = false); bool isArchAny() const; std::vector validate() const; diff --git a/libpkg/data/storage.cpp b/libpkg/data/storage.cpp index ab18e6e..ba0c4ab 100644 --- a/libpkg/data/storage.cpp +++ b/libpkg/data/storage.cpp @@ -129,6 +129,11 @@ auto StorageCache::retrieve(Storage & return retrieve(storage, nullptr, entryName); } +/*! + * \brief Stores the specified \a entry. + * \remarks The entry may exist or may not exist. A lookup for an existing entry is done to take over + * deps/provides from the existing entry if it makes sense. + */ template auto StorageCache::store(Storage &storage, RWTxn &txn, const std::shared_ptr &entry) -> StoreResult { @@ -149,6 +154,7 @@ auto StorageCache::store(Storage &sto entry->addDepsAndProvidesFromOtherPackage(*res.oldEntry); } lock.unlock(); + // check for package in storage if (!res.oldEntry) { res.oldEntry = std::make_shared(); @@ -158,8 +164,10 @@ auto StorageCache::store(Storage &sto res.oldEntry.reset(); } } + // update package in storage res.id = txn.put(*entry, res.id); + // update cache entry lock = std::unique_lock(m_mutex); if (cacheEntry) { @@ -169,10 +177,31 @@ auto StorageCache::store(Storage &sto } cacheEntry->entry = entry; lock.unlock(); + res.updated = true; return res; } +/*! + * \brief Stores the specified \a entry with the specified \a storageID. + * \remarks This is used to update an existing entry with a known ID. + */ +template +auto StorageCache::store( + Storage &storage, RWTxn &txn, StorageID storageID, const std::shared_ptr &entry) -> void +{ + // update package in storage + const auto id = txn.put(*entry, storageID); + + // update cache entry + using CacheEntry = typename Entries::StorageEntry; + using CacheRef = typename Entries::Ref; + const auto ref = CacheRef(storage, entry); + const auto lock = std::unique_lock(m_mutex); + const auto cacheEntry = &m_entries.insert(CacheEntry(ref, id)); + cacheEntry->entry = entry; +} + template bool StorageCache::invalidate(Storage &storage, const std::string &entryName) { diff --git a/libpkg/data/storagegeneric.h b/libpkg/data/storagegeneric.h index 756ff01..4feecf8 100644 --- a/libpkg/data/storagegeneric.h +++ b/libpkg/data/storagegeneric.h @@ -152,6 +152,7 @@ template SpecType retrieve(Storage &storage, RWTxn *, const std::string &entryName); SpecType retrieve(Storage &storage, const std::string &entryName); StoreResult store(Storage &storage, RWTxn &txn, const std::shared_ptr &entry); + void store(Storage &storage, RWTxn &txn, StorageID storageID, const std::shared_ptr &entry); bool invalidate(Storage &storage, const std::string &entryName); bool invalidateCacheOnly(Storage &storage, const std::string &entryName); void clear(Storage &storage); diff --git a/librepomgr/buildactions/reloadlibrarydependencies.cpp b/librepomgr/buildactions/reloadlibrarydependencies.cpp index 51f953e..ce5f577 100644 --- a/librepomgr/buildactions/reloadlibrarydependencies.cpp +++ b/librepomgr/buildactions/reloadlibrarydependencies.cpp @@ -377,19 +377,19 @@ void ReloadLibraryDependencies::loadPackageInfoFromContents() } // find the package in the database again const auto [packageID, existingPackage] = updater.findPackageWithID(package.info.name); - if (!existingPackage) { - continue; // the package has been removed while we were loading package contents + // skip if the package has been removed meanwhile if it it does no longer match what's in the database + if (!existingPackage || !existingPackage->canDepsAndProvidesFromOtherPackage(package.info)) { + continue; } // add the dependencies/provides to the existing package - if (!existingPackage->addDepsAndProvidesFromOtherPackage(package.info)) { - continue; // the package does no longer match what's in the database - } + updater.beginUpdate(packageID, existingPackage); + existingPackage->addDepsAndProvidesFromOtherPackage(package.info, true); // update timestamp so we can skip this package on the next run if (existingPackage->timestamp < package.lastModified) { existingPackage->timestamp = package.lastModified; } // add the new dependencies on database-level - updater.update(existingPackage); + updater.endUpdate(packageID, existingPackage); ++counter; } updater.commit(); diff --git a/librepomgr/tests/buildactions.cpp b/librepomgr/tests/buildactions.cpp index 9326607..893c60c 100644 --- a/librepomgr/tests/buildactions.cpp +++ b/librepomgr/tests/buildactions.cpp @@ -314,6 +314,7 @@ void BuildActionsTests::testParsingInfoFromPkgFiles() auto &fooDb = config.databases[0]; auto &barDb = config.databases[1]; const auto harfbuzz = LibPkg::Package::fromPkgFileName("mingw-w64-harfbuzz-1.4.2-1-any.pkg.tar.xz"); + harfbuzz->libprovides = { "harfbuzzlibrary.so" }; const auto harfbuzzID = fooDb.updatePackage(harfbuzz); const auto syncthingtray = LibPkg::Package::fromPkgFileName("syncthingtray-0.6.2-1-x86_64.pkg.tar.xz"); const auto syncthingtrayID = fooDb.updatePackage(syncthingtray); @@ -322,6 +323,14 @@ void BuildActionsTests::testParsingInfoFromPkgFiles() barDb.updatePackage(cmake); CPPUNIT_ASSERT_EQUAL_MESSAGE("origin", LibPkg::PackageOrigin::PackageFileName, cmake->origin); barDb.localPkgDir = directory(testFilePath("repo/bar/cmake-3.8.2-1-x86_64.pkg.tar.xz")); + auto harfbuzzLibraryPresent = false; + fooDb.providingPackages("harfbuzzlibrary.so", false, [&](LibPkg::StorageID id, const std::shared_ptr &package) { + CPPUNIT_ASSERT_EQUAL(harfbuzzID, id); + CPPUNIT_ASSERT_EQUAL(harfbuzz->name, package->name); + harfbuzzLibraryPresent = true; + return true; + }); + CPPUNIT_ASSERT_MESSAGE("harfbuzz found via \"harfbuzzlibrary.so\" before reload", harfbuzzLibraryPresent); auto buildAction = std::make_shared(0, &m_setup); auto reloadLibDependencies = ReloadLibraryDependencies(m_setup, buildAction); @@ -345,6 +354,11 @@ void BuildActionsTests::testParsingInfoFromPkgFiles() CPPUNIT_ASSERT_EQUAL(1_st, pkgsProvidingLibSyncthingConnector.size()); CPPUNIT_ASSERT_EQUAL(syncthingtray->name, pkgsProvidingLibSyncthingConnector.front().pkg->name); CPPUNIT_ASSERT_EQUAL(syncthingtrayID, pkgsProvidingLibSyncthingConnector.front().id); + + fooDb.providingPackages("harfbuzzlibrary.so", false, [&](LibPkg::StorageID, const std::shared_ptr &) { + CPPUNIT_FAIL("harfbuzz still found via \"harfbuzzlibrary.so\" after reload"); + return true; + }); } /*!